From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_MSPIKE_H2 autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 32599 invoked from network); 5 Oct 2022 01:01:05 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 5 Oct 2022 01:01:05 -0000 Received: (qmail 7643 invoked by uid 550); 5 Oct 2022 01:01:01 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 7605 invoked from network); 5 Oct 2022 01:01:00 -0000 Date: Tue, 4 Oct 2022 21:00:46 -0400 From: Rich Felker To: musl@lists.openwall.com Message-ID: <20221005010044.GR29905@brightrain.aerifal.cx> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] 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