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]

Message-ID: <4DDCB0D2.2050505@gmail.com>
Date:	Wed, 25 May 2011 09:33:38 +0200
From:	Nicolas de Pesloüan 
	<nicolas.2p.debian@...il.com>
To:	Jay Vosburgh <fubar@...ibm.com>
CC:	Neil Horman <nhorman@...driver.com>,
	Andy Gospodarek <andy@...yhouse.net>, netdev@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH] bonding: prevent deadlock on slave store with alb mode

Le 25/05/2011 02:34, Jay Vosburgh a écrit :

> 	One alternative is to simply permit all option changes while the
> bond is down, then commit then as a set when the bond is brought up (or
> fail the open or adjust options if the options are invalid).  More or
> less what you suggest, except that the "commit" is the ifup.
>
> 	Even that is a substantial rework of how things are done now,
> since as you point out, options today take effect more or less in real
> time.

Agreed.

Setting an option while the master is down should only syntax-check the option value and store the 
value for later use (at the time we bring the bond up). Setting an option while the master is up 
should take effect in realtime if possible/appropriate or store it and issue a warning saying one 
need to down+up the bond to commit the change.

>> This also begs the question, is it or is it not safe to enslave devices while
>> the bond is down?  Clearly from the bug report its unsafe, and I don't know what
>> other (if any) conditions exist that cause problems when doing this (be that a
>> deadlock, panic or simply undefined or unexpected behavior).  If its really
>> unsafe, then issuing a warning seems incorrect, we shouldn't allow user space to
>> cause things like this, and as such, we should return an error.  If it is safe
>> (generally) and this is an isolated bug, then we should probably remove the
>> warning.  But to just issue a vague 'This might do bad things' warning seems
>> wrong in either case.
>
> 	Agreed.  I think it (conceptually) should be safe to add or
> remove slaves when the bond is down, and bonding shouldn't complain
> about it.

Are there any reasons not to consider enslavement as a normal option with the same processing as 
other options (only do real enslavement at the time we bring the master up)?

	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