Re: [PATCH net] net: bridge: fix memleak in br_add_if()

[kernel test robot <lkp@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 7948 bytes --]

Hi Yang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]

url:    https://github.com/0day-ci/linux/commits/Yang-Yingliang/net-bridge-fix-memleak-in-br_add_if/20210809-105706
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 3125f26c514826077f2a4490b75e9b1c7a644c42
config: parisc-randconfig-r013-20210809 (attached as .config)
compiler: hppa-linux-gcc (GCC) 10.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/1c7f031037ef82751f6ec66247c59d87c301b732
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yang-Yingliang/net-bridge-fix-memleak-in-br_add_if/20210809-105706
        git checkout 1c7f031037ef82751f6ec66247c59d87c301b732
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=parisc SHELL=/bin/bash net/bridge/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   net/bridge/br_if.c: In function 'br_add_if':
>> net/bridge/br_if.c:619:16: error: 'struct net_bridge_port' has no member named 'mcast_stats'
     619 |   free_percpu(p->mcast_stats);
         |                ^~
   net/bridge/br_if.c:733:15: error: 'struct net_bridge_port' has no member named 'mcast_stats'
     733 |  free_percpu(p->mcast_stats);
         |               ^~


vim +619 net/bridge/br_if.c

   557	
   558	/* called with RTNL */
   559	int br_add_if(struct net_bridge *br, struct net_device *dev,
   560		      struct netlink_ext_ack *extack)
   561	{
   562		struct net_bridge_port *p;
   563		int err = 0;
   564		unsigned br_hr, dev_hr;
   565		bool changed_addr, fdb_synced = false;
   566	
   567		/* Don't allow bridging non-ethernet like devices. */
   568		if ((dev->flags & IFF_LOOPBACK) ||
   569		    dev->type != ARPHRD_ETHER || dev->addr_len != ETH_ALEN ||
   570		    !is_valid_ether_addr(dev->dev_addr))
   571			return -EINVAL;
   572	
   573		/* Also don't allow bridging of net devices that are DSA masters, since
   574		 * the bridge layer rx_handler prevents the DSA fake ethertype handler
   575		 * to be invoked, so we don't get the chance to strip off and parse the
   576		 * DSA switch tag protocol header (the bridge layer just returns
   577		 * RX_HANDLER_CONSUMED, stopping RX processing for these frames).
   578		 * The only case where that would not be an issue is when bridging can
   579		 * already be offloaded, such as when the DSA master is itself a DSA
   580		 * or plain switchdev port, and is bridged only with other ports from
   581		 * the same hardware device.
   582		 */
   583		if (netdev_uses_dsa(dev)) {
   584			list_for_each_entry(p, &br->port_list, list) {
   585				if (!netdev_port_same_parent_id(dev, p->dev)) {
   586					NL_SET_ERR_MSG(extack,
   587						       "Cannot do software bridging with a DSA master");
   588					return -EINVAL;
   589				}
   590			}
   591		}
   592	
   593		/* No bridging of bridges */
   594		if (dev->netdev_ops->ndo_start_xmit == br_dev_xmit) {
   595			NL_SET_ERR_MSG(extack,
   596				       "Can not enslave a bridge to a bridge");
   597			return -ELOOP;
   598		}
   599	
   600		/* Device has master upper dev */
   601		if (netdev_master_upper_dev_get(dev))
   602			return -EBUSY;
   603	
   604		/* No bridging devices that dislike that (e.g. wireless) */
   605		if (dev->priv_flags & IFF_DONT_BRIDGE) {
   606			NL_SET_ERR_MSG(extack,
   607				       "Device does not allow enslaving to a bridge");
   608			return -EOPNOTSUPP;
   609		}
   610	
   611		p = new_nbp(br, dev);
   612		if (IS_ERR(p))
   613			return PTR_ERR(p);
   614	
   615		call_netdevice_notifiers(NETDEV_JOIN, dev);
   616	
   617		err = dev_set_allmulti(dev, 1);
   618		if (err) {
 > 619			free_percpu(p->mcast_stats);
   620			kfree(p);	/* kobject not yet init'd, manually free */
   621			goto err1;
   622		}
   623	
   624		err = kobject_init_and_add(&p->kobj, &brport_ktype, &(dev->dev.kobj),
   625					   SYSFS_BRIDGE_PORT_ATTR);
   626		if (err)
   627			goto err2;
   628	
   629		err = br_sysfs_addif(p);
   630		if (err)
   631			goto err2;
   632	
   633		err = br_netpoll_enable(p);
   634		if (err)
   635			goto err3;
   636	
   637		err = netdev_rx_handler_register(dev, br_get_rx_handler(dev), p);
   638		if (err)
   639			goto err4;
   640	
   641		dev->priv_flags |= IFF_BRIDGE_PORT;
   642	
   643		err = netdev_master_upper_dev_link(dev, br->dev, NULL, NULL, extack);
   644		if (err)
   645			goto err5;
   646	
   647		err = nbp_switchdev_mark_set(p);
   648		if (err)
   649			goto err6;
   650	
   651		dev_disable_lro(dev);
   652	
   653		list_add_rcu(&p->list, &br->port_list);
   654	
   655		nbp_update_port_count(br);
   656		if (!br_promisc_port(p) && (p->dev->priv_flags & IFF_UNICAST_FLT)) {
   657			/* When updating the port count we also update all ports'
   658			 * promiscuous mode.
   659			 * A port leaving promiscuous mode normally gets the bridge's
   660			 * fdb synced to the unicast filter (if supported), however,
   661			 * `br_port_clear_promisc` does not distinguish between
   662			 * non-promiscuous ports and *new* ports, so we need to
   663			 * sync explicitly here.
   664			 */
   665			fdb_synced = br_fdb_sync_static(br, p) == 0;
   666			if (!fdb_synced)
   667				netdev_err(dev, "failed to sync bridge static fdb addresses to this port\n");
   668		}
   669	
   670		netdev_update_features(br->dev);
   671	
   672		br_hr = br->dev->needed_headroom;
   673		dev_hr = netdev_get_fwd_headroom(dev);
   674		if (br_hr < dev_hr)
   675			update_headroom(br, dev_hr);
   676		else
   677			netdev_set_rx_headroom(dev, br_hr);
   678	
   679		if (br_fdb_insert(br, p, dev->dev_addr, 0))
   680			netdev_err(dev, "failed insert local address bridge forwarding table\n");
   681	
   682		if (br->dev->addr_assign_type != NET_ADDR_SET) {
   683			/* Ask for permission to use this MAC address now, even if we
   684			 * don't end up choosing it below.
   685			 */
   686			err = dev_pre_changeaddr_notify(br->dev, dev->dev_addr, extack);
   687			if (err)
   688				goto err7;
   689		}
   690	
   691		err = nbp_vlan_init(p, extack);
   692		if (err) {
   693			netdev_err(dev, "failed to initialize vlan filtering on this port\n");
   694			goto err7;
   695		}
   696	
   697		spin_lock_bh(&br->lock);
   698		changed_addr = br_stp_recalculate_bridge_id(br);
   699	
   700		if (netif_running(dev) && netif_oper_up(dev) &&
   701		    (br->dev->flags & IFF_UP))
   702			br_stp_enable_port(p);
   703		spin_unlock_bh(&br->lock);
   704	
   705		br_ifinfo_notify(RTM_NEWLINK, NULL, p);
   706	
   707		if (changed_addr)
   708			call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
   709	
   710		br_mtu_auto_adjust(br);
   711		br_set_gso_limits(br);
   712	
   713		kobject_uevent(&p->kobj, KOBJ_ADD);
   714	
   715		return 0;
   716	
   717	err7:
   718		if (fdb_synced)
   719			br_fdb_unsync_static(br, p);
   720		list_del_rcu(&p->list);
   721		br_fdb_delete_by_port(br, p, 0, 1);
   722		nbp_update_port_count(br);
   723	err6:
   724		netdev_upper_dev_unlink(dev, br->dev);
   725	err5:
   726		dev->priv_flags &= ~IFF_BRIDGE_PORT;
   727		netdev_rx_handler_unregister(dev);
   728	err4:
   729		br_netpoll_disable(p);
   730	err3:
   731		sysfs_remove_link(br->ifobj, p->dev->name);
   732	err2:
   733		free_percpu(p->mcast_stats);
   734		kobject_put(&p->kobj);
   735		dev_set_allmulti(dev, -1);
   736	err1:
   737		dev_put(dev);
   738		return err;
   739	}
   740	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30268 bytes --]