netdev - Re: netlink circular locking dependency

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: <4857B9E5.3070102@trash.net>
Date:	Tue, 17 Jun 2008 15:19:33 +0200
From:	Patrick McHardy <kaber@...sh.net>
To:	Thomas Graf <tgraf@...g.ch>
CC:	Jarek Poplawski <jarkao2@...il.com>,
	Marcel Holtmann <marcel@...tmann.org>, netdev@...r.kernel.org,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: netlink circular locking dependency

Thomas Graf wrote:
> * Jarek Poplawski <jarkao2@...il.com> 2008-06-17 13:09
>> On Tue, Jun 17, 2008 at 02:50:19PM +0200, Patrick McHardy wrote:
>> ...
>>> Thanks for testing. Unfortunately the module unload races look
>>> more complicated to fix and I'm busy with other things, so it
>>> would great if someone else could fix this.
>> Patrick, I wonder if simply adding an additional mutex e.g.
>> genl_lock_table() around all the rest (after your patch) genl_locks
>> could be enough until some major rework. This should prevent any
>> new races and there are no lockups, I guess?
> 
> It would be wise to redo the locking alltogether while we're fixing
> this. Decouple message serialization from register/unregister
> operations would be a nice thing f.e.

Agreed.

> I'll have a look if none beats me to it.

While whoever is at this, there are a few similar races I
wanted to fix for a long time, but never got to it.

- the rtnl_register/rtnl_unregister function don't perform any
locking, so they might get called while unregistration is
in progress

- the individual ops are registered one at a time, which could
theoretically lead to some strange behaviour where a subset of
the ops can already be used, but others return EOPNOTSUPP

- netlink_dump_start usage in rtnetlink has the exact same
race as genetlink with my patch

For 1. and 2., it seems the best way would be to register the
entire set of ops (encapsulated in a struct) at once and hold
the rtnl_mutex during that. For 3., the fix would probably
identical to the genetlink fix.

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