Re: [PATCH net v3] bonding: fix slave_cnt leak on XDP error paths

On Tue, Sep 08, 2026 at 07:09:51PM +0000, netdev-bot+sashiko@kernel.org wrote:

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 1 · High: 0 · Medium: 0 (1) · Low: 0

- [Critical] Newly introduced slab out-of-bounds write in
  bond_update_slave_arr(): moving `WRITE_ONCE(bond->slave_cnt,…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the XDP
  error paths the patch touches are still incompletely unwound: the…

--

bonding: fix slave_cnt leak on XDP error paths

This moves the WRITE_ONCE(bond->slave_cnt, bond->slave_cnt + 1) in
bond_enslave() from just above netdev_compute_master_upper_features() down
to after the XDP setup block, so that the three XDP failure exits which
"goto err_sysfs_del" no longer leave slave_cnt incremented.  The increment
is placed just before the bond_update_slave_arr() call.

Link to v2: https://lore.kernel.org/r/20260903-bond_slave_cnt-v2-1-02e27304ca36@kylinos.cn (local)
Link to v1: https://lore.kernel.org/r/20260902-bond_slave_cnt-v1-1-36e95bf4a6ff@kylinos.cn (local)

quoted

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a9bff7663eeca..1514a8ba0888d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c

@@ -2284,7 +2284,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 		}
 	}
 
-	WRITE_ONCE(bond->slave_cnt, bond->slave_cnt + 1);

       ^^^^

[Severity: Critical]
Can removing the increment from here overflow all_slaves->arr[] in
bond_update_slave_arr()?

This is a false positive report. The bond->slave_cnt update is not removed.
It's moved just before bond_update_slave_arr().

Thanks
Hangbin

quoted

 	netdev_compute_master_upper_features(bond->dev, true);
 	bond_set_carrier(bond);
 

@@ -2332,6 +2331,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 			bpf_prog_inc(bond->xdp_prog);
 	}

       ^^^^

[Severity: Medium]
This isn't a bug introduced by this patch, but the XDP error exits being
adjusted here are still only partially unwound.

The three "goto err_sysfs_del" cases in this block ("Slave does not support
XDP", "Slave has XDP program loaded" and the dev_xdp_propagate() failure)
enter a chain of labels:

	err_sysfs_del -> err_upper_unlink -> err_unregister -> err_detach ->
	err_close -> err_restore_mac -> err_restore_mtu -> err_free ->
	err_undo_flags

None of those labels calls dev_set_promiscuity(slave_dev, -1),
dev_set_allmulti(slave_dev, -1), bond_hw_addr_flush() or
bond_slave_ns_maddrs_del().  Those undo steps exist only in
__bond_release_one():

drivers/net/bonding/bond_main.c:__bond_release_one() {
	...
		if (old_flags & IFF_ALLMULTI)
			dev_set_allmulti(slave_dev, -1);

		if (old_flags & IFF_UP)
			bond_hw_addr_flush(bond_dev, slave_dev);
	...
}

which a failed enslave never reaches.  So when the bond is IFF_PROMISC,
IFF_ALLMULTI or IFF_UP and has an xdp_prog attached, does a failed enslave
leave the candidate device with a leaked promiscuity/allmulti refcount, the
bond's synced mc/uc addresses (plus the lacpdu multicast address in 8023AD
mode) still installed, and the NS-target multicast addresses added by
bond_slave_ns_maddrs_add() still present?  Since the device is no longer a
slave, is there any path left that can drop those refcounts?

Maybe flush multicast addr after dev_close(slave_dev)?
Anyway, this is need another patch.

quoted

 
+	/* Increase the slave count before rebuilding the slave arrays. */
+	WRITE_ONCE(bond->slave_cnt, bond->slave_cnt + 1);
+
 	/* broadcast mode uses the all_slaves to loop through slaves. */
 	if (bond_mode_can_use_xmit_hash(bond) ||
 	    BOND_MODE(bond) == BOND_MODE_BROADCAST)

Thanks
Hangbin