Subject: Re: [PATCH net] team: fix possible recursive locking when add slaves

From: Jiri Pirko <[email-masked]>
Date: Fri, 19 Apr 2019 07:55:29 +0200

Wed, Apr 17, 2019 at 02:12:11PM CEST, [email-masked] wrote:
>If we add a bond device which is already the master of the team interface,
>we will hold the team->lock in team_add_slave() first and then request the
>lock in team_set_mac_address() again. The functions are called like:
>
>- team_add_slave()
> - team_port_add()
>   - team_port_enter()
>     - team_modeop_port_enter()
>       - __set_port_dev_addr()
>         - dev_set_mac_address()
>           - bond_set_mac_address()
>             - dev_set_mac_address()
>  	       - team_set_mac_address
>
>Although team_upper_dev_link() would check the upper devices but it is
>called too late. Fix it by adding a checking before processing the slave.
>
>Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
>Signed-off-by: Hangbin Liu <[email-masked]>
>---
> drivers/net/team/team.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>index 9ce61b019aad..8313c4f2b635 100644
>--- a/drivers/net/team/team.c
>+++ b/drivers/net/team/team.c
>@@ -1156,6 +1156,13 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
> 		return -EINVAL;
> 	}
> 
>+	if (netdev_has_upper_dev(dev, port_dev)) {
>+		NL_SET_ERR_MSG(extack, "Device is already an upper device of the team interface");
>+		netdev_err(dev, "Device %s is already an upper device "
>+				"of the team interface\n", portname);

No wraps in strings please.
Otherwise, this looks fine. Feel free to add my ack:
Acked-by: Jiri Pirko <[email-masked]>

>+		return -EBUSY;
>+	}
>+
> 	if (port_dev->features & NETIF_F_VLAN_CHALLENGED &&
> 	    vlan_uses_dev(dev)) {
> 		NL_SET_ERR_MSG(extack, "Device is VLAN challenged and team device has VLAN set up");
>-- 
>2.19.2
>