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.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_MSPIKE_H2 autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 15810 invoked from network); 5 Oct 2022 12:10:26 -0000 Received: from second.openwall.net (193.110.157.125) by inbox.vuxu.org with ESMTPUTF8; 5 Oct 2022 12:10:26 -0000 Received: (qmail 15805 invoked by uid 550); 5 Oct 2022 12:10:22 -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 15764 invoked from network); 5 Oct 2022 12:10:21 -0000 DKIM-Filter: OpenDKIM Filter v2.11.0 mail.ispras.ru 4E7C342291F5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ispras.ru; s=default; t=1664971809; bh=K2MnFC5xIdKmU1IkX4N/Gc01yu33ZPsukWMzIC9D3s8=; h=Date:From:To:Subject:Reply-To:In-Reply-To:References:From; b=oWGsdWXG71135KyF1GuBDrTn8OuNrz/4Y9Yo3ef14IlQC7cTUZ0YY7zZ0VKVyUtUL QLMGdPwrM/AmgGVJQDgYV+SmE1e7UARrCzgyJX3u0Z1Uzw5YdEMb93CpMKaX/UU4qG m81+yPFb9c2NY1DqsQumV0TgH4kXqunPFSZnZLo0= MIME-Version: 1.0 Date: Wed, 05 Oct 2022 15:10:09 +0300 From: Alexey Izbyshev To: musl@lists.openwall.com Mail-Followup-To: musl@lists.openwall.com In-Reply-To: <20221005010044.GR29905@brightrain.aerifal.cx> References: <20221005010044.GR29905@brightrain.aerifal.cx> User-Agent: Roundcube Webmail/1.4.4 Message-ID: X-Sender: izbyshev@ispras.ru Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [musl] Illegal killlock skipping when transitioning to single-threaded state On 2022-10-05 04:00, Rich Felker wrote: > 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. > This was my conclusion as well back when I looked at it before sending the report. I was initially concerned about whether reordering with a_store(&self->detach_state, DT_EXITED) could cause an unwanted observable change (pthread_tryjoin_np() returning EBUSY after a pthread function acting on tid like pthread_getschedparam() returns ESRCH), but no, pthread_tryjoin_np() will block/trap if the thread is not DT_JOINABLE. > 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. > I'm not sure why using a lock-free list is messy, it seems like a perfect fit here to me. However, doesn't __dl_vseterr() use the libc-internal allocator after 34952fe5de44a833370cbe87b63fb8eec61466d7? If so, the problem that freebuf_queue was originally solving doesn't exist anymore. We still can't call the allocator after __tl_lock(), but maybe this whole free deferral approach can be reconsidered? Alexey