[PATCH v2] team: fix promisc/allmulti leak in team_port_add error path

[PATCH v2] team: fix promisc/allmulti leak in team_port_add error path

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

team_port_add() increments the port's promiscuity and allmulti
counts before calling team_dev_type_check_change(). If the latter
fails, the error path does not undo these increments, leaving the
port in promiscuous and allmulti mode even though it was not
successfully added to the team.

Undo the promiscuity increment under err_set_slave_allmulti and the
allmulti increment under err_set_dev_type, so each error label only
reverts what was successfully applied, matching the cleanup performed
by team_port_del().

With the promiscuity rollback now living under err_set_slave_allmulti,
the inline dev_set_promiscuity(port_dev, -1) in the dev_set_allmulti()
failure branch becomes redundant: that branch already jumps to
err_set_slave_allmulti, which undoes the promiscuity increment. Drop
the inline call to avoid doing it twice.

Found by source code audit.

Fixes: 0ae9cfc454ea ("team: Move team device type change at the end of team_port_add")
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
Diff with v1:
1. Remove the dev_set_promiscuity(port_dev, -1); in if (dev->flags & IFF_ALLMULTI) and move it under here.
2. change Fixes tag

 drivers/net/team/team_core.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index 957cc78..cfb833c 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -1365,11 +1365,8 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
 	/* set allmulti level to new slave */
 	if (dev->flags & IFF_ALLMULTI) {
 		err = dev_set_allmulti(port_dev, 1);
-		if (err) {
-			if (dev->flags & IFF_PROMISC)
-				dev_set_promiscuity(port_dev, -1);
+		if (err)
 			goto err_set_slave_allmulti;
-		}
 	}
 
 	err = team_dev_type_check_change(dev, port_dev);
@@ -1397,7 +1394,11 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
 	return 0;
 
 err_set_dev_type:
+	if (dev->flags & IFF_ALLMULTI)
+		dev_set_allmulti(port_dev, -1);
 err_set_slave_allmulti:
+	if (dev->flags & IFF_PROMISC)
+		dev_set_promiscuity(port_dev, -1);
 err_set_slave_promisc:
 	__team_option_inst_del_port(team, port);
 
-- 
2.9.4

Re: [PATCH v2] team: fix promisc/allmulti leak in team_port_add error path

[Hangbin Liu]

On Wed, Nov 04, 2026 at 09:52:17AM +0800, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> team_port_add() increments the port's promiscuity and allmulti
> counts before calling team_dev_type_check_change(). If the latter
> fails, the error path does not undo these increments, leaving the
> port in promiscuous and allmulti mode even though it was not
> successfully added to the team.
> 
> Undo the promiscuity increment under err_set_slave_allmulti and the
> allmulti increment under err_set_dev_type, so each error label only
> reverts what was successfully applied, matching the cleanup performed
> by team_port_del().
> 
> With the promiscuity rollback now living under err_set_slave_allmulti,
> the inline dev_set_promiscuity(port_dev, -1) in the dev_set_allmulti()
> failure branch becomes redundant: that branch already jumps to
> err_set_slave_allmulti, which undoes the promiscuity increment. Drop
> the inline call to avoid doing it twice.
> 
> Found by source code audit.
> 
> Fixes: 0ae9cfc454ea ("team: Move team device type change at the end of team_port_add")
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
> Diff with v1:
> 1. Remove the dev_set_promiscuity(port_dev, -1); in if (dev->flags & IFF_ALLMULTI) and move it under here.
> 2. change Fixes tag
> 
>  drivers/net/team/team_core.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
> index 957cc78..cfb833c 100644
> --- a/drivers/net/team/team_core.c
> +++ b/drivers/net/team/team_core.c
> @@ -1365,11 +1365,8 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
>  	/* set allmulti level to new slave */
>  	if (dev->flags & IFF_ALLMULTI) {
>  		err = dev_set_allmulti(port_dev, 1);
> -		if (err) {
> -			if (dev->flags & IFF_PROMISC)
> -				dev_set_promiscuity(port_dev, -1);
> +		if (err)
>  			goto err_set_slave_allmulti;
> -		}
>  	}
>  
>  	err = team_dev_type_check_change(dev, port_dev);
> @@ -1397,7 +1394,11 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
>  	return 0;
>  
>  err_set_dev_type:
> +	if (dev->flags & IFF_ALLMULTI)
> +		dev_set_allmulti(port_dev, -1);
>  err_set_slave_allmulti:
> +	if (dev->flags & IFF_PROMISC)
> +		dev_set_promiscuity(port_dev, -1);
>  err_set_slave_promisc:
>  	__team_option_inst_del_port(team, port);
>  
> -- 
> 2.9.4
> 

Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>