netdev - Re: [PATCH net] bonding: fix slave stuck in BOND_LINK

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] [day] [month] [year] [list]

Date:   Wed, 08 Nov 2017 16:08:17 +0900 (KST)
From:   David Miller <davem@...emloft.net>
To:     jay.vosburgh@...onical.com
Cc:     netdev@...r.kernel.org, alexandre.sidorenko@....com,
        maheshb@...gle.com, jarod@...hat.com, vfalico@...il.com,
        andy@...yhouse.net
Subject: Re: [PATCH net] bonding: fix slave stuck in BOND_LINK_FAIL state

From: Jay Vosburgh <jay.vosburgh@...onical.com>
Date: Tue, 07 Nov 2017 19:50:07 +0900

> 	The bonding miimon logic has a flaw, in that a failure of the
> rtnl_trylock can cause a slave to become permanently stuck in
> BOND_LINK_FAIL state.
> 
> 	The sequence of events to cause this is as follows:
> 
> 	1) bond_miimon_inspect finds that a slave's link is down, and so
> calls bond_propose_link_state, setting slave->new_link_state to
> BOND_LINK_FAIL, then sets slave->new_link to BOND_LINK_DOWN and returns
> non-zero.
> 
> 	2) In bond_mii_monitor, the rtnl_trylock fails, and the timer is
> rescheduled.  No change is committed.
> 
> 	3) bond_miimon_inspect is called again, but this time the slave
> from step 1 has recovered.  slave->new_link is reset to NOCHANGE, and, as
> slave->link was never changed, the switch enters the BOND_LINK_UP case,
> and does nothing.  The pending BOND_LINK_FAIL state from step 1 remains
> pending, as new_link_state is not reset.
> 
> 	4) The state from step 3 persists until another slave changes link
> state and causes bond_miimon_inspect to return non-zero.  At this point,
> the BOND_LINK_FAIL state change on the slave from steps 1-3 is committed,
> and the slave will remain stuck in BOND_LINK_FAIL state even though it
> is actually link up.
> 
> 	The remedy for this is to initialize new_link_state on each entry
> to bond_miimon_inspect, as is already done with new_link.
> 
> Reported-by: Alex Sidorenko <alexandre.sidorenko@....com>
> Reviewed-by: Jarod Wilson <jarod@...hat.com>
> Signed-off-by: Jay Vosburgh <jay.vosburgh@...onical.com>
> Fixes: fb9eb899a6dc ("bonding: handle link transition from FAIL to UP correctly")

Applied and queued up for -stable.

As discussed with some others here at netdev... rtnl_trylock() really needs
to be re-evaluated if not removed completely.