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.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,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 DB4A721F71 for ; Sat, 20 Jan 2024 12:29:42 +0100 (CET) Received: (qmail 19515 invoked by uid 550); 20 Jan 2024 11:27:42 -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 19483 invoked from network); 20 Jan 2024 11:27:42 -0000 DKIM-Filter: OpenDKIM Filter v2.11.0 mail.ispras.ru 05B1240F1DF9 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ispras.ru; s=default; t=1705750169; bh=H5/PaLiuioMf5pvcBwhBIcP3QPm31C75AGkFLDO7tYU=; h=Date:From:To:Subject:Reply-To:In-Reply-To:References:From; b=Tyl767nn8FfpvJy+GMGa5G6OsOjRcKP1yYIWnTw5fG46zodvqaCkYuOHrEhdqJoXP 7X6XSUbRuMJ8nW3PE759S7qu+9VRi4hrgAj/EecOrIbY9lDwiyS1LfVMSmD0nPJoON XAuiVXhAZPkf+IWqiFwo4ifgHMECa//DdarVCjwE= MIME-Version: 1.0 Date: Sat, 20 Jan 2024 14:29:28 +0300 From: Alexey Izbyshev To: musl@lists.openwall.com Mail-Followup-To: musl@lists.openwall.com In-Reply-To: References: User-Agent: Roundcube Webmail/1.4.13 Message-ID: <4da16095016feeccc01d23b3b29533f5@ispras.ru> X-Sender: izbyshev@ispras.ru Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [musl] [PATCH] Split fork and abort locks On 2024-01-20 11:12, Markus Wichmann wrote: > Hi all, > > a while ago I had noticed that __abort_lock was being taken in some > functions that have nothing to do with SIGABRT. Namely in the forking > functions. Investigating this a bit, I noticed that __abort_lock had > become dual purpose. But this is a code smell. > > Actually, there are several locks that have expanded in scope a bit > since their introduction. At least the ptc lock (__inhibit_ptc() et > al.) > deserves a closer look later on as well. Seems to me like in case of > the > default stack size, that lock is used simply because an rwlock was > needed and this one was around. > > The corruption in this case probably came from posix_spawn(). That > function takes the abort lock, because, as a baffling comment above the > lock statement tells us, this guards against SIGABRT disposition > changing. This is because abort() changes the disposition without > calling sigaction(), so a handler would not be noted in the handler > set. > > However, actually posix_spawn() does not need to care about this. The > handler set is strictly additive, so all the signals it contains /may/ > have a handler. And abort() removes a handler. In the worst case, the > handler set will spuriously contain SIGABRT, which is a condition the > child needs to check for anyway. > > And a concurrent sigaction() call from the application establishing a > handler is no different for SIGABRT than for any other signal. That is > handled by retrieving the handler set in the child, when everything is > fixed since the child is single-threaded. For the same reason, there > cannot be a concurrent call to abort() in the child. > The problem that __abort_lock solves is that a child process created by fork/_Fork/clone/posix_spawn should not observe SIGABRT disposition reset if abort is called by the parent concurrently with the child creation. For example, if the initial SIGABRT disposition is SIG_IGN, and one thread of a program calls posix_spawn while another thread calls abort, without __abort_lock the child could inherit SIG_DFL disposition. This is not related to maintaining handler_set used for posix_spawn optimization. A separate reason for why removing __abort_lock LOCK/UNLOCK from clone.c and _Fork.c (as done in your patch) is wrong is because they take part in creation of a consistent execution state in the child (the child should be able to call abort and not deadlock). Alexey