netdev - Re: [PATCH] rtnl: Simplify ASSERT

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: <46FE7019.4030705@trash.net>
Date:	Sat, 29 Sep 2007 17:32:41 +0200
From:	Patrick McHardy <kaber@...sh.net>
To:	Herbert Xu <herbert@...dor.apana.org.au>
CC:	"Eric W. Biederman" <ebiederm@...ssion.com>, davem@...emloft.net,
	netdev@...r.kernel.org
Subject: Re: [PATCH] rtnl: Simplify ASSERT_RTNL

Herbert Xu wrote:
> Eric W. Biederman <ebiederm@...ssion.com> wrote:
> 
>>Currently we have the call path:
>>macvlan_open -> dev_unicast_add -> __dev_set_rx_mode ->
>>       __dev_set_promiscuity -> ASSERT_RTNL -> mutex_trylock
>>
>>When mutex debugging is on taking a mutex complains if we are not
>>allowed to sleep.  At that point we have called netif_tx_lock_bh
>>so we are clearly not allowed to sleep.  Arguably this is not a
>>problem for mutex_trylock.
> 
> 
> Actually holding the TX lock here is a bug.  We're going to
> call down into the hardware with __dev_set_promiscuity, which
> may sleep (think USB NICs), so we definitely shouldn't be holding
> any spin locks.
> 
> Patrick, could we avoid taking the TX lock here somehow?

For unicast addresses its not strictly necessary since they may
only be changed under the RTNL anyway. The reason why it takes
the tx_lock is for consistency with multicast address handling,
which can't rely on the RTNL since IPv6 changes them from
BH context. The idea was that the ->set_rx_mode function should
handle both secondary unicast and multicast addresses for
simplicity.

But I don't understand the problem, if this doesn't work for
unicast addresses, why does it work for multicast addresses
and ->set_multicast_list? I had a quick look at some USB
network drivers but couldn't spot anything special ..
-
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