netdev - Re: [PATCH] bonding: prevent deadlock on slave store with alb mode

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:	Tue, 24 May 2011 23:02:50 +0200
From:	Nicolas de Pesloüan 
	<nicolas.2p.debian@...il.com>
To:	Neil Horman <nhorman@...driver.com>
CC:	Andy Gospodarek <andy@...yhouse.net>, netdev@...r.kernel.org,
	Jay Vosburgh <fubar@...ibm.com>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH] bonding: prevent deadlock on slave store with alb mode

Le 24/05/2011 22:51, Nicolas de Pesloüan a écrit :
> Le 24/05/2011 22:37, Neil Horman a écrit :
>
>>>>> + return -EINVAL;
>>>
>>> This will turn a warning into an error.
>>>
>> Yes, because it should have been an error all along.
>>
>>> This warning existed for long, but never caused the bonding setup to
>>> fail. This patch cause some regression for user space. For example,
>>> current ifenslave-2.6 package in Debian doesn't ensure bond is UP
>>> before enslaving, because this was never required.
>>>
>> Thats not a regression, thats the kernel returning an error where it should have
>> done so all along. Just because a utility got away with it for awhile and it
>> didn't always cause a lockup, doesn't grandfather that application in to a
>> situation where the kernel has to support its broken behavior in perpituity.
>>
>> Besides, iirc, the ifsenslave utility still uses the ioctl path, which this
>> patch doesn't touch, so ifenslave is currently unaffected (although I should
>> look in the ioctl path to see if we have already added such a check, lest you be
>> able to deadlock your system as previously indicated using that tool).
>
> Unfortunately, no. Recent versions of ifenslave-2.6 on Debian don't use ioctl (ifenslave binary)
> anymore, but only sysfs.
>
> Documentation/bonding.txt should be updated to reflect this change.
> pr_warning should be changed to pr_ err.
> Bonding version should be bumped.
>
> Anyway, I will fix this package, but I suspect there exist many user scripts that don't ensure bond
> is up before enslaving.

Well, still thinking about it...

On Ubuntu, due to the usage of upstart, the slaves are ifup before the master. On Debian, this is 
also true for hotplug slaves. I wonder whether this patch may cause troubles, because the master 
will have to be up before calling ifup for it.

	Nicolas.
--
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