Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221005010044.GR29905@brightrain.aerifal.cx>
Date: Tue, 4 Oct 2022 21:00:46 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Illegal killlock skipping when transitioning to
 single-threaded state

On Wed, Sep 07, 2022 at 03:46:53AM +0300, Alexey Izbyshev wrote:
> Reordering the "libc.need_locks = -1" assignment and
> UNLOCK(E->killlock) and providing a store barrier between them
> should fix the issue.

Back to this, because it's immediately actionable without resolving
the aarch64 atomics issue:

Do you have something in mind for how this reordering is done, since
there are other intervening steps that are potentially ordered with
respect to either or both? I don't think there is actually any
ordering constraint at all on the unlocking of killlock (with the
accompanying assignment self->tid=0 kept with it) except that it be
past the point where we are committed to the thread terminating
without executing any more application code. So my leaning would be to
move this block from the end of pthread_exit up to right after the
point-of-no-return comment.

Unfortunately while reading this I found another bug, this time a lock
order one. __dl_thread_cleanup() takes a lock while the thread list
lock is already held, but fork takes these in the opposite order. I
think the lock here could be dropped and replaced with an atomic-cas
list head, but that's rather messy and I'm open to other ideas.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.