Re: [PATCH] nptl: Fix deadlock on atfork handler which calls dlclose (BZ#24595)

[Florian Weimer <fweimer at redhat dot com>]

* Adhemerval Zanella:

> On 23/05/2019 16:50, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> The solution sounds correct, but I don't have a strong opinion if this
>>> is really an improvement over a recursive lock plus a linked list.  It 
>>> potentially adds 'free' calls in fork for multithread mode if list needs 
>>> to be deallocated. Also, since the locks is internal to register-atfork.c
>>> we might have a better control to make the exported interfaces not 
>>> deadlock.
>> 
>> Recursive locks do not prevent deadlocks if the callback launches
>> threads.  I'm not sure if this can be considered a stretch.  If
>> there's dlclose involved, their might also be dlopen, and threads
>> launched from an ELF constructor and so on.
>
> That's why I said the core issue is atfork handlers are really
> underspecified, and maybe it would be better if we document more
> thoughtfully what we really aim to support by atfork handlers.

That's also true.

>> Furthermore, I'm not sure if the implementation of __unregister_atfork
>> is correct.  I think you have a potential use-after-free issue if the
>> callback deregisters a fork handler.

> The possible way I can think of is if an atfork handler from a module
> try to dlclose itself and I am not sure how valid it is.

dlclose'ing itself is not really valid, I think.  It can only work if
the dlclose call is a tail call.

__unregister_atfork still adds a use-after-free issue, even with the
copy-on-write list: If a dlclose unregisters a fork handler, we will
still attempt to call it subsequently if it was on the list before.  No
data race is involved if the dlclose call happens itself in a fork
handler (so we are back to the territory of the bug).

So the simple reference counting scheme I proposed is probably not the
right solution, and we need a different data structure.

Thanks,
Florian