Re: [musl] Re: MT fork and key_lock in pthread_key_create.c

[Rich Felker <dalias@libc.org>]

On Thu, Oct 06, 2022 at 04:04:48PM -0400, Jeffrey Walton wrote:
> On Thu, Oct 6, 2022 at 3:21 PM Rich Felker <dalias@libc.org> wrote:
> > On Thu, Oct 06, 2022 at 10:02:11AM +0300, Alexey Izbyshev wrote:
> > > On 2022-10-06 09:37, Alexey Izbyshev wrote:
> > > >...
> > > >Also, I looked at how __aio_atfork() handles a similar case with
> > > >maplock, and it seems wrong. It takes the read lock and then simply
> > > >unlocks it both in the parent and in the child. But if there were
> > > >other holders of the read lock at the time of fork(), the lock won't
> > > >end up in the unlocked state in the child. It should probably be
> > > >completely nulled-out in the child instead.
> > > >
> > > Looking at aio further, I don't understand how it's supposed to work
> > > with MT fork at all. __aio_atfork() is called in _Fork() when the
> > > allocator locks are already held. Meanwhile another thread could be
> > > stuck in __aio_get_queue() holding maplock in exclusive mode while
> > > trying to allocate, resulting in deadlock.
> >
> > Indeed, this is messy and I don't think it makes sense to be doing
> > this at all. The child is just going to throw away the state so the
> > parent shouldn't need to synchronize at all, but if we walk the
> > multi-level map[] table in the child after async fork, it's possible
> > that the contents seen are inconsistent, even that the pointers are
> > only half-written or something.
> >
> > I see a few possible solutions:
> >
> > 1. Just set map = 0 in the child and leak the memory. This is not
> >    going to matter unless you're doing multiple generations of fork
> >    with aio anyway.
> 
> This may make security testing and evaluation trickier, like when
> using -fanalyze=memory.
> 
> I think it is better to work with the tools nowadays.

These allocations are immutable/permanent-once-allocated anyway in the
parent. The only difference would be potentially getting an additional
round of them in the child after forking. "Leak detection" tooling
already needs to be aware that there will be some one-time permanent
allocations that will/can never be freed.

Rich