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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190419055529.GD2247@nanopsycho>
Date:   Fri, 19 Apr 2019 07:55:29 +0200
From:   Jiri Pirko <jiri@...nulli.us>
To:     Hangbin Liu <liuhangbin@...il.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        Xin Long <lucien.xin@...il.com>
Subject: Re: [PATCH net] team: fix possible recursive locking when add slaves

Wed, Apr 17, 2019 at 02:12:11PM CEST, liuhangbin@...il.com 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 <liuhangbin@...il.com>
>---
> 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 <jiri@...lanox.com>


>+		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
>