Re: netlink circular locking dependency

[Patrick McHardy <kaber@trash.net>]

Thomas Graf wrote:
> * Jarek Poplawski <jarkao2@gmail.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.