netdev - Re: RTNL: assertion failed at net/core/dev.c (4494) and RTNL: assertion failed at net/core/rtnetlink.c (940)

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: <7882.1391822502@death.nxdomain>
Date:	Fri, 07 Feb 2014 17:21:42 -0800
From:	Jay Vosburgh <fubar@...ibm.com>
To:	"sfeldma@...ulusnetworks.com" <sfeldma@...ulusnetworks.com>
Cc:	Cong Wang <cwang@...pensource.com>,
	Thomas Glanzmann <thomas@...nzmann.de>,
	Eric Dumazet <eric.dumazet@...il.com>,
	Veaceslav Falico <vfalico@...hat.com>, andy@...yhouse.net,
	Jiří Pírko <jiri@...nulli.us>,
	netdev <netdev@...r.kernel.org>
Subject: Re: RTNL: assertion failed at net/core/dev.c (4494) and RTNL: assertion failed at net/core/rtnetlink.c (940)

Jay Vosburgh <fubar@...ibm.com> wrote:

>
>Cong Wang <cwang@...pensource.com> wrote:
>
>>On Thu, Feb 6, 2014 at 2:07 PM, Jay Vosburgh <fubar@...ibm.com> wrote:
>>> Jay Vosburgh <fubar@...ibm.com> wrote:
>>>
>>>>Cong Wang <cwang@...pensource.com> wrote:
>>>>
>>>>
>>>>       That would eliminate the warning, but is suboptimal.  Acquiring
>>>>RTNL is not necessary on the vast majority of state machine runs
>>>>(because no state changes take place, i.e., no ports are disabled or
>>>>enabled).  The above change would add 10 round trips per second to RTNL,
>>>>which seems excessive.
>>>>
>>>>       Also, we cannot unconditionally acquire RTNL in this function,
>>>>as it would race with the call to cancel_delayed_work_sync from
>>>>bond_close (via bond_work_cancel_all).
>>
>>OK.
>>
>>>
>>>         Thought of one more problem: we can't hold a regular lock while
>>> calling rtmsg_ifinfo, as it may sleep in alloc_skb.  The rtmsg_ifinfo
>>> call has to be RTNL and nothing else.
>>>
>>
>>s/GFP_KERNEL/GFP_ATOMIC/
>
>	Yah, that would help with extra locks, but not totally solve
>things.  I'm looking around, and seeing a number of other places that
>will end up at one of these rtmsg_ifinfo calls with incorrect locking:
>
>	bond_ab_arp_probe calls via bond_set_slave_active_flags and
>bond_set_slave_inactive_flags without RTNL.
>
>	bond_change_active_slave calls via bond_set_slave_inactive_flags
>and bond_set_slave_active_flags with other locks held, and maybe without
>RTNL; I'm not sure if bond_option_active_slave_set holds RTNL when it
>calls bond_select_active_slave.
>
>	bond_open calls via bond_set_slave_active_flags and
>bond_set_slave_inactive_flags with RTNL, but also with other locks held.
>
>	bond_loadbalance_arp_mon calls bond_set_active_slave and
>bond_set_backup_slave without RTNL.
>
>	This is in addition to the cases in the 802.3ad code from
>__enable_port and __disable_port calls.

	Just an update in case anybody else is looking into this, and
some questions for Scott.

	Acquiring RTNL for the __enable_port and __disable_port cases is
difficult, as those calls generally already hold the state machine lock,
and cannot unconditionally call rtnl_lock because either they already
hold RTNL (for calls via bond_3ad_unbind_slave) or due to the potential
for deadlock with bond_3ad_adapter_speed_changed,
bond_3ad_adapter_duplex_changed, bond_3ad_link_change, or
bond_3ad_update_lacp_rate.  All four of those are called with RTNL held,
and acquire the state machine lock second.  The calling contexts for
__enable_port and __disable_port already hold the state machine lock,
and may or may not need RTNL.

	Scott: you added these calls, so can you explain what they're
for?  I'm asking for two reasons:

	First, if they do not occur synchronously is it going to be a
problem?  E.g., for the 802.3ad case, if the rtmsg_ifinfo is called
either at the end of the state machine run, or for non-state machine
events, at the next run of the state machine (which is every 100 ms),
would that be a problem?  Setting a flag in the slave somewhere that an
rtmsg_ifinfo is needed should be doable for the 802.3ad case.

	Second, what do the messages mean?  That the slave is now
"active and usable"?  I'm asking because I suspect the bond_ab_arp_probe
usage wherein it adjusts the flags and curr_active_slave should not
actually call rtmsg_ifinfo, as the slave there is not really "up."
What's going on there is that the ARP monitor cycles through each slave
one by one, and tests to see if that slave works.  If it does work, then
it is set as the active elsewhere in the monitor code.  This function
adjusts the flags so that the ARP monitor will treat the "testing" slave
as "active" for purposes of determining whether or not it is up.  I
suspect this adjustment to the flags should not actually generate an
rtmsg_ifinfo.

	I think the remaining cases can be dealt with, but clarification
on the above two questions would be very helpful.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.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