From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.1 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: from second.openwall.net (second.openwall.net [193.110.157.125]) by inbox.vuxu.org (Postfix) with SMTP id 6AEBD2162E for ; Sun, 21 Jan 2024 18:03:01 +0100 (CET) Received: (qmail 1816 invoked by uid 550); 21 Jan 2024 17:00:58 -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 1772 invoked from network); 21 Jan 2024 17:00:58 -0000 Date: Sun, 21 Jan 2024 12:03:03 -0500 From: Rich Felker To: julien.voisin@dustri.org Cc: musl@lists.openwall.com Message-ID: <20240121170302.GA4163@brightrain.aerifal.cx> References: <20240109190726.GO4163@brightrain.aerifal.cx> <20240121034301.GZ4163@brightrain.aerifal.cx> <820837e29ea605142a934e672d670fbbd9d44cbd@dustri.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <820837e29ea605142a934e672d670fbbd9d44cbd@dustri.org> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] Protect pthreads' mutexes against use-after-destroy On Sun, Jan 21, 2024 at 12:06:14PM +0000, julien.voisin@dustri.org wrote: > > Draft attached in case anyone wants to play with it. This could > > probably be something we could consider to adopt. > > Couldn't a macro like `#define mutex_is_destroyed (!(m->_m_type & 8) && (m->_m_lock == 0x3fffffff)` be > used instead? Or at least named constants instead of `8` and `0x3fffffff`.. Maybe something like that, but that's a change I'd like to make in a consistent uniform way for all of the uses of magic numbers in the mutex implementation. Just introducing it in a single place like this doesn't really help readability; in some ways it makes it less readable since you can't see how it's interacting with the other tests. If doing it, I think it would probably make more sense not to have that predicate macro, but instead something like: if (own == M_UNRECOVERABLE && !(m->_m_type & MT_ROBUST)) because seeing the individual parts is relevant to understanding: > Also, the code-style seems inconsistent: > > ``` > + if (own == 0x3fffffff) { > + /* Catch use-after-destroy */ > + if (!(type & 8)) a_crash(); > + return ENOTRECOVERABLE; > + } > ``` > > vs > > ``` > + /* Catch use-after-destroy */ > + if (own == 0x3fffffff && !(type & 8)) a_crash(); > return EPERM; > ``` > > Both are the same check, yet only one has both conditions in a single `if`. These are decision trees on what to do in exceptional cases. The aim is not to present a consistent "style" between two functions that do different things and have different decision trees for how to act. Rich