netdev - Re: [PATCH net] bonding: fix bond dev flags after convert to arphrd

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Wed, 15 Jul 2015 13:32:33 -0700
From:	Jay Vosburgh <jay.vosburgh@...onical.com>
To:	Nikolay Aleksandrov <razor@...ckwall.org>
cc:	netdev@...r.kernel.org, monis@...taire.com,
	gospo@...ulusnetworks.com, vfalico@...il.com, davem@...emloft.net,
	Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
Subject: Re: [PATCH net] bonding: fix bond dev flags after convert to arphrd_ether

Nikolay Aleksandrov <razor@...ckwall.org> wrote:

>From: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
>
>If a bonding device enslaves devices != arphrd_ether it'll change types
>and if later these devices are released, it can enslave an arphrd_ether
>device and switch back calling ether_setup() which resets dev->flags to
>IFF_BROADCAST|IFF_MULTICAST and clears IFF_MASTER which then could lead
>to many different bugs. This bug seems to have been there since the
>introduction of ether_setup() in bond_enslave().

	I thought the hack-around for the non-ethernet device problem
was that, for non-ARPHRD_ETHER devices, the bonding master device is
automatically destroyed when the last slave is released.

	This (well, this sort of thing) originally came up with IPoIB
devices needing different ops that would disappear if the ipoib module
was unloaded, so the bond master was deliberately unregistered when the
last slave was released.

	Looking at the code, at first glance this appears to still be
the case: bond_slave_netdev_event calls bond_release_and_destroy for !=
ARPHRD_ETHER, which in turn should unregister the bond itself if there
are no slaves left.  Is this no longer working as intended?

	-J


>Signed-off-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
>Fixes: e36b9d16c6a6 ("bonding: clean muticast addresses when device changes type")
>---
> drivers/net/bonding/bond_main.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 317a49480475..8ba119896e55 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -1368,6 +1368,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> 				bond_setup_by_slave(bond_dev, slave_dev);
> 			else {
> 				ether_setup(bond_dev);
>+				bond_dev->flags |= IFF_MASTER;
> 				bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
> 			}
> 
>-- 
>1.9.3
>

---
	-Jay Vosburgh, jay.vosburgh@...onical.com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html