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

From: kernel test robot
Date: Mon Aug 09 2021 - 05:54:07 EST

Next message: Manivannan Sadhasivam: "Re: linux-next: build failure after merge of the mhi tree"
Previous message: Robert Richter: "Re: [PATCH v3] drivers/edac/edac_mc: Remove all strcpy() uses"
In reply to: kernel test robot: "Re: [PATCH net] net: bridge: fix memleak in br_add_if()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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: hexagon-randconfig-r034-20210809 (attached as .config)
compiler: clang version 12.0.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=clang make.cross O=build_dir ARCH=hexagon SHELL=/bin/bash net/bridge/

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

All errors (new ones prefixed by >>):

>> net/bridge/br_if.c:619:18: error: no member named 'mcast_stats' in 'struct net_bridge_port'
free_percpu(p->mcast_stats);
~ ^
net/bridge/br_if.c:733:17: error: no member named 'mcast_stats' in 'struct net_bridge_port'
free_percpu(p->mcast_stats);
~ ^
2 errors generated.

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@xxxxxxxxxxxx

Attachment: .config.gz
Description: application/gzip

Next message: Manivannan Sadhasivam: "Re: linux-next: build failure after merge of the mhi tree"
Previous message: Robert Richter: "Re: [PATCH v3] drivers/edac/edac_mc: Remove all strcpy() uses"
In reply to: kernel test robot: "Re: [PATCH net] net: bridge: fix memleak in br_add_if()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]