netdev - Re: IPv4/IPv6 sysctl unregistration deadlock

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: <m14oyhis31.fsf@fess.ebiederm.org>
Date:	Wed, 25 Feb 2009 23:18:42 -0800
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Herbert Xu <herbert@...dor.apana.org.au>
Cc:	Patrick McHardy <kaber@...sh.net>,
	Linux Netdev List <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: IPv4/IPv6 sysctl unregistration deadlock

Herbert Xu <herbert@...dor.apana.org.au> writes:

> On Wed, Feb 25, 2009 at 10:10:33PM -0800, Eric W. Biederman wrote:
>>
>> How does adding a rename operation to sysctl sound?
>
> Yes that would definitely help.  Of course for the unregister case
> we'd still need either an async removal or a no-op as Patrick
> suggested.

After having reread the thread and looking at the code I think
I understand what is happening.

sysctl, proc, and sysfs all need to wait until there are no
more users before their unregister operation succeeds.  So that
we can guarantee that it is safe to remove a module that provides
the callback function.

Currently ndo_stop, NETDEV_DOWN, unlist_netdevice and I don't
know how much other code is run from unregister_netdevice
with the rtnl lock.  If we do an asynchronous unregister
we need to ensure that entire code path is safe without
rtnl_lock.  And we would need to run the unregister work
from rtnl_lock.

Ugh.  netdev_store() and a few other functions in net-sysfs.c
take rtnl_lock.  The instance in netdev_store appears to date
back to 21 May 2003 sometime during 2.5.

So this is an old problem that we are just noticing now. Ugh.

Currently rtnl_lock() protects the netdevice_notifier_chain.
So it appears we need to hold rtnl_lock().

Which leads me to conclude either we need to completely rewrite the
locking rules for the networking stack, or we need to teach the sysfs,
sysctl, and proc how to grab a subsystem lock around a callback.

We already do this for netlink with netlink_create_kernel.

So I guess we need a variants of:
register_sysctl_table, proc_create, and class_create_file.

What a pain, but at least it looks like it can work.

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