netdev - Re: [PATCH next v5 2/6] bonding: implement bond_poll

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]

Message-ID: <CAF2d9jhxZZ65mVyV0TcXt=YkKmbe3EKhEkEhx6LT+TA9h_Vt=g@mail.gmail.com>
Date:	Wed, 18 Feb 2015 21:06:25 -0800
From:	Mahesh Bandewar <maheshb@...gle.com>
To:	Maciej Żenczykowski <maze@...gle.com>
Cc:	Nikolay Aleksandrov <nikolay@...hat.com>,
	Jay Vosburgh <j.vosburgh@...il.com>,
	Andy Gospodarek <andy@...yhouse.net>,
	Veaceslav Falico <vfalico@...il.com>,
	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH next v5 2/6] bonding: implement bond_poll_controller()

On Wed, Feb 18, 2015 at 6:26 PM, Maciej Żenczykowski <maze@...gle.com> wrote:
>>>> Hi Mahesh,
>>>> I should've explained more in my review, you cannot sleep in
>>>> bond_poll_controller() so you cannot acquire rtnl like that. I was thinking
>>>> more about using rcu and switching to the _rcu version of
>>>> bond_for_each_slave instead.
>>>>
>>> That makes sense. The path that triggered this netpoll() could have
>>> been holding the rtnl  itself and this would be a problem. I think
>>> using the _rcu variant of the slave iterator is a good idea, my bad!
>>>
>> ... however we cannot use the _rcu variant either since there is the
>> netpoll mutex (ni->dev_lock)!
>> The fact that we are here itself means that something bad had happened
>> and trying to take additional lock(s) would complicate the situation
>> further.
>
> I think you might be incorrectly assuming that we only get here on
> kernel crashes,
> upstream (netconsole) [may] violate[s] this assumption.
>
> (background: internally we don't consider netconsole production
> worthy, but we do
> 'abuse' the netpoll support framework to generate a minimal network
> dump on kernel crash,
> in which case if we crash or otherwise violate assumptions while
> crashing, we're not any worse off...)
>
I think irrespective of how we use it, there are few contradictory things here -
(a) If it can't sleep in netpoll() (which I agree with) then that
netpoll mutex does not make sense. But if it can then rtnl isn't a
wrong thing.
(b) If the netpoll mutex is right then slaves can not be iterated with
rcu protection.
(c) If we takeout the netpoll-mutex, then everything seems right but
can we really do that?

I'll take this patch out of this series for the time being and let
other patches proceed while I think about this patch some more.

--mahesh..

> - Maciej
--
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