From patchwork Tue Jul 23 13:41:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 1963902 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=g5aSrtVD; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=libc-alpha-bounces~incoming=patchwork.ozlabs.org@sourceware.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WSyyf71lNz1yZw for ; Tue, 23 Jul 2024 23:43:06 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 34063386183A for ; Tue, 23 Jul 2024 13:43:05 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-pf1-x429.google.com (mail-pf1-x429.google.com [IPv6:2607:f8b0:4864:20::429]) by sourceware.org (Postfix) with ESMTPS id 0F5673858430 for ; Tue, 23 Jul 2024 13:42:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0F5673858430 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 0F5673858430 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::429 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1721742166; cv=none; b=CzKGwj7RQKTGPwuccvAKuI5111q0LVSBQp8DAjOWVwg2/DtCSANDmpSK+7M9pNduOGtHmzwd9FglwxkfOEu/Du2u9VUrD1n9JhwBuj4S2nXrP/fcKZev3wIN/o99uLWI3xbxDlqWq/ppT+MNhRKhyOvd8vFL2ypEtk40BiXZL88= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1721742166; c=relaxed/simple; bh=wBsqYH9j7Z2iDBntj3fLVORFEjlHPONB0qFSWAbvf90=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=NIL8ko4wPyQEK5VpgcJTJ+hvu7iv3+6GunYFG8WAubAUFL5WHZMOyhtCK18HmCHeYOHEihNbeAR9lsZMPm3I7N1FBRFzH645Ew0tMjDEv7twS5E9aGkmyGZxHxZXZp4DqZ8dwnaBW0YwfEBTVbHv/nNzOg8ztk6JxTmJxJhyyEM= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x429.google.com with SMTP id d2e1a72fcca58-70d316f0060so1072088b3a.1 for ; Tue, 23 Jul 2024 06:42:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1721742162; x=1722346962; darn=sourceware.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:from:to:cc:subject:date:message-id :reply-to; bh=ytNO9jfF8YKOUnAdOGwrT3mHNJ0sPueas9IsGMvCOog=; b=g5aSrtVDGVD4TVq9KtV+JNOiZizD/MAy9bO2kYP+gMx92FmfEIozsKvp8V6XjNVGB8 rqaZVg0pTDZtLjq4SE4V7VwkbSN2kIVtAAfxWbpuvG90uKkCd5whEGrCyBF7TeMyN6Yj 5+0njtNCa8mgwoHu3Xdi1GmeBVYUYfz9E1PfL6TSVo12qlHq2MrdAYIab6/U4tevDbT+ TM4FVDG/50EdquifpwhiSmzAmlzzvsDVJ8FiBr1uxbDUDj8htSXK7ExEULOEk1PN1TxE i0Np79xF5qaAogamZnkjkPeOmbpPQF0zH08pBUB+5rdfWZT7s+yJ8pgziW3SnqVJ3Wxn 8BYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721742162; x=1722346962; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ytNO9jfF8YKOUnAdOGwrT3mHNJ0sPueas9IsGMvCOog=; b=qMNAmXCdHRqzOCIPq4k3XFGBNi3JqD0ysX1OrM1i0baE6oVNwplq6X6IzvlLbrEu91 zMzI5Sh5VKGMnO3SfKa8oOeTpyFSuh0S4/VhUK+sRny/IicJpobjshephZLCW8VLeJ2S T/tEpqk04iEgz0zmILHu9A38Rae4X7TpETFhlY6WMNTaiyGetrKISKhHt7K2uKYOp+qL TvEsWnn7DFZ1UJxUBvkLuqnGbPcYhX4RJEkE01FFTb5Ryyb6mRRLFc405V/qyj6Ms3yU L9hEeOV8m6LAsbehjYysiaSlBe0V5mxHGSpTooKc8lKSzts3gbbMz5NxwEF7m0s67Y3v JNXg== X-Gm-Message-State: AOJu0YxpIQabXsbATPq5KguaZXL0V4gyqYCP1GO+vQ5KhUlR0XOg8ILx jc4V2E0OxGqfCjJaG+6QcbLr4hlA6eTbbtqc3fz+sXtTh4M3uzvLMZSVP9Rh4mzyiTMpY9xNUC+ I X-Google-Smtp-Source: AGHT+IH91z6RyPqz+oqCN6pOh72Gek58etPpaRsbOdMOnxNd4ZVisOSUNAdMq7oupPYTu36iUcS5lw== X-Received: by 2002:a05:6a00:1a8c:b0:70d:2fef:2ca0 with SMTP id d2e1a72fcca58-70e8074fda4mr3392610b3a.2.1721742162449; Tue, 23 Jul 2024 06:42:42 -0700 (PDT) Received: from mandiga.. ([2804:1b3:a7c0:842a:1aa9:89f1:3b40:9ac]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-79f0af5e635sm6321269a12.22.2024.07.23.06.42.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Jul 2024 06:42:41 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org, Carlos O'Donell , Florian Weimer Subject: [PATCH v2 1/2] setjmp: Use BSD sematic as default for setjmp Date: Tue, 23 Jul 2024 10:41:48 -0300 Message-ID: <20240723134235.1520483-2-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240723134235.1520483-1-adhemerval.zanella@linaro.org> References: <20240723134235.1520483-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces~incoming=patchwork.ozlabs.org@sourceware.org POSIX relaxed the relation of setjmp/longjmp and the signal mask save/restore, meaning that setjmp does not require to be routed to _setjmp to be standard compliant. This is done to avoid breakage of SIGABRT handlers, since to fully make abort AS-safe, it is required to remove the recurisve lock used to unblock SIGABRT prior raised the signal. Also, it allows caller to actually use setjmp, since from 7011c2622fe3e10a29dbe74f06aaebd07710127d the symbol is unconditionally routed to _setjmp. Checked on x86_64-linux-gnu. --- NEWS | 4 +++- manual/setjmp.texi | 14 ++++---------- nptl/pthread_create.c | 3 ++- setjmp/setjmp.h | 5 ----- sysdeps/nptl/libc_start_call_main.h | 3 ++- 5 files changed, 11 insertions(+), 18 deletions(-) diff --git a/NEWS b/NEWS index 2f67f3f315..a30fb748d6 100644 --- a/NEWS +++ b/NEWS @@ -17,7 +17,9 @@ Deprecated and removed features, and other changes affecting compatibility: Changes to build and runtime requirements: - [Add changes to build and runtime requirements here] +* The setjmp now uses the BSD semantic as default, where the signal mask + is saved. This is required to avoid the break of SIGABRT handler for + the async-signal-safe abort implementation. Security related changes: diff --git a/manual/setjmp.texi b/manual/setjmp.texi index 7092a0dde2..f2d82a2f33 100644 --- a/manual/setjmp.texi +++ b/manual/setjmp.texi @@ -189,16 +189,10 @@ them @code{volatile}. @section Non-Local Exits and Signals In BSD Unix systems, @code{setjmp} and @code{longjmp} also save and -restore the set of blocked signals; see @ref{Blocking Signals}. However, -the POSIX.1 standard requires @code{setjmp} and @code{longjmp} not to -change the set of blocked signals, and provides an additional pair of -functions (@code{sigsetjmp} and @code{siglongjmp}) to get the BSD -behavior. - -The behavior of @code{setjmp} and @code{longjmp} in @theglibc{} is -controlled by feature test macros; see @ref{Feature Test Macros}. The -default in @theglibc{} is the POSIX.1 behavior rather than the BSD -behavior. +restore the set of blocked signals; see @ref{Blocking Signals}, while +on @w{System V} they will not. POSIX does not specify the relation of +@code{setjmp} and @code{longjmp} to signal mask. The default in +@theglibc{} is the BSD behavior. The facilities in this section are declared in the header file @file{setjmp.h}. diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c index 1d3665d5ed..527cb9017d 100644 --- a/nptl/pthread_create.c +++ b/nptl/pthread_create.c @@ -402,7 +402,8 @@ start_thread (void *arg) the saved signal mask), so that is a false positive. */ DIAG_IGNORE_NEEDS_COMMENT (11, "-Wstringop-overflow="); #endif - not_first_call = setjmp ((struct __jmp_buf_tag *) unwind_buf.cancel_jmp_buf); + not_first_call = __sigsetjmp ( + (struct __jmp_buf_tag *) unwind_buf.cancel_jmp_buf, 0); DIAG_POP_NEEDS_COMMENT; /* No previous handlers. NB: This must be done after setjmp since the diff --git a/setjmp/setjmp.h b/setjmp/setjmp.h index 1309c6210d..98efa3e863 100644 --- a/setjmp/setjmp.h +++ b/setjmp/setjmp.h @@ -44,11 +44,6 @@ extern int __sigsetjmp (struct __jmp_buf_tag __env[1], int __savemask) __THROWNL Return 0. */ extern int _setjmp (struct __jmp_buf_tag __env[1]) __THROWNL; -/* Do not save the signal mask. This is equivalent to the `_setjmp' - BSD function. */ -#define setjmp(env) _setjmp (env) - - /* Jump to the environment saved in ENV, making the `setjmp' call there return VAL, or 1 if VAL is 0. */ extern void longjmp (struct __jmp_buf_tag __env[1], int __val) diff --git a/sysdeps/nptl/libc_start_call_main.h b/sysdeps/nptl/libc_start_call_main.h index 26ac8dd53b..c41ca6843b 100644 --- a/sysdeps/nptl/libc_start_call_main.h +++ b/sysdeps/nptl/libc_start_call_main.h @@ -41,7 +41,8 @@ __libc_start_call_main (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL), the saved signal mask), so that is a false positive. */ DIAG_IGNORE_NEEDS_COMMENT (11, "-Wstringop-overflow="); #endif - not_first_call = setjmp ((struct __jmp_buf_tag *) unwind_buf.cancel_jmp_buf); + not_first_call = __sigsetjmp ( + (struct __jmp_buf_tag *) unwind_buf.cancel_jmp_buf, 0); DIAG_POP_NEEDS_COMMENT; if (__glibc_likely (! not_first_call)) { From patchwork Tue Jul 23 13:41:49 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 1963903 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=dB/WBo1l; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=libc-alpha-bounces~incoming=patchwork.ozlabs.org@sourceware.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4WSyyn3gKdz1yZw for ; Tue, 23 Jul 2024 23:43:13 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C23453861807 for ; Tue, 23 Jul 2024 13:43:11 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-pg1-x531.google.com (mail-pg1-x531.google.com [IPv6:2607:f8b0:4864:20::531]) by sourceware.org (Postfix) with ESMTPS id A35043860C3D for ; Tue, 23 Jul 2024 13:42:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A35043860C3D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A35043860C3D Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::531 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1721742171; cv=none; b=BdP0ssfXxcKXp8QJs1fzWhdKdke5ymciI4Id8KHTF/pQ4yA4MCSIyiUWhL33dsG4MXS9Mytwx3CjfCAZGLI6RmbpBOE/SFPkVhukCA7aXBav+UiXq1IBMXE6tMDh5rTqO/yWhwtIb1ZkKdgf2X/A+08/7RxEE43+f0voZ8U9bVE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1721742171; c=relaxed/simple; bh=QouTkD0mlrbQKGUsiNqS99pmc7jT6hYEqzNYcscndaE=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=J3HKXzhz53RWeVX1IkUBRDnpoG0Vw3u7C+ngG9d3G6Hjrwsk4O73gc5VW5qL0wdLpEFU6H7g1cnOiYkJycfk769pNtPdzLI0OzO9hXZxKj+6zEYYFMlJO6/d06JyBR2NuUDswVj0f51J778gKtQUCGuApFok9cJNMq22OT1aUIY= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pg1-x531.google.com with SMTP id 41be03b00d2f7-7a1843b4cdbso533812a12.2 for ; Tue, 23 Jul 2024 06:42:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1721742165; x=1722346965; darn=sourceware.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:from:to:cc:subject:date:message-id :reply-to; bh=7D3XIcfo075H1z9wef0LVG2tLYZ2i53ZHYJSnNHjhek=; b=dB/WBo1llf8NYpFK3D+MxnMXGzOwLhXvkvJw0+cD1dSDy6jW7LDQm47Y/LkV264u+Y hsoyFjYwEvTM4DCNj5YggkBrItnvVIOWit+eJ0AR3CmRLGCrQmct1h1avuiTUKmHeK7u Z56CenDrIhHMe0h9itc6eYwI+A+otL91uRZggZeLJwf6pbA7WS646w/kFCpl24Z704vJ nmgNm2drT/EbL7UPeah78JZVaKqBPs/3tPRZbURe8djvrnv6MwbOzNxHV46xHTu3M0dW J6mb42MLM9gyuLORTulpzsS2Z/YquoaApvBB0p395agxYMcrjJK0C+YIHZuYngbUkWUr QgXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721742165; x=1722346965; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=7D3XIcfo075H1z9wef0LVG2tLYZ2i53ZHYJSnNHjhek=; b=eitvSogEPiuwgVsqbWbf0WSo2GmPvRL3lxiw990J7Dr9jmrp4AmIE+cHF1kX30liIQ +8wCQCbQ/mqbIpn3HjeBM0Ewe2c8AcZBm0mDVodd/afeiV6LmqlxqBkgrH0vSgw1EyUN Zy3hRhh+BvB5DhYwMCvCHlHFT1yP6Pm2aQ6LBFmaYWqYSTZTCKvuTIm+Aq9xpc0WEwPI 8DoF7qxxJfiilOPFSLUfTSi6WSnHa6QYs6WnP2PSh7ruTuTbWrvuNnCaUTVerWr6xK7m tWQpn5vJOLsZR0PVUl8Cx00dFwG8UZklbg4g8N5O20uZaAanW7rRKKB0VEZLBx1mazfn hrhw== X-Gm-Message-State: AOJu0YytcQkzJC0KWhAiPkC6MIk0L2DKPfvSAtCD12bUK4jo2SQap4PX 8v1n/Rvm/rCiarljA/iOx9x2XxWvBbPpltkfg1NlUdHb4xiJe2phHLbJngIHFcYJ+IOjhCWeYBW E X-Google-Smtp-Source: AGHT+IHTcfoOypPbZHzWxJmZXeMtiF6ZjWo2er25XMGMZL27BC11mRjTgVtpMhGEqO41Ck6E4GR9sw== X-Received: by 2002:a05:6a21:1807:b0:1c3:b231:33f3 with SMTP id adf61e73a8af0-1c4285e3d04mr8231518637.38.1721742164934; Tue, 23 Jul 2024 06:42:44 -0700 (PDT) Received: from mandiga.. ([2804:1b3:a7c0:842a:1aa9:89f1:3b40:9ac]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-79f0af5e635sm6321269a12.22.2024.07.23.06.42.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Jul 2024 06:42:44 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org, Carlos O'Donell , Florian Weimer Subject: [PATCH v2 2/2] stdlib: Make abort/_Exit AS-safe (BZ 26275) Date: Tue, 23 Jul 2024 10:41:49 -0300 Message-ID: <20240723134235.1520483-3-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240723134235.1520483-1-adhemerval.zanella@linaro.org> References: <20240723134235.1520483-1-adhemerval.zanella@linaro.org> MIME-Version: 1.0 X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces~incoming=patchwork.ozlabs.org@sourceware.org The recursive lock used on abort does not synchronize with new process creation (either by fork-like interfaces or posix_spawn ones), nor it is reinitialized after fork(). Also, the SIGABRT unblock before raise shows another race condition, where a fork or posix_spawn call by another thread just after the recursive lock release and before the SIGABRT raise might create programs with a non-expected signal mask. To fix the AS-safe, the raise issue without changing the process signal mask, and an AS-safe lock is used if a SIGABRT is installed or the process is blocked or ignored. The the signal mask change removal, there is no need to use a recursive lock. The lock is also on both _Fork and posix_spawn, to avoid the spawn process to see the abort handler as SIG_DFL. The posix_spawn possible issue is if the caller sets a SIG_IGN for SIGABRT, calls abort, and another thread issues posix_spawn just after the sigaction returns. With default options (not setting POSIX_SPAWN_SETSIGDEF), the process can still see SIG_DFL for SIGABRT, where it should be SIG_IGN. The fallback is also simplified, there is no need to use a loop of ABORT_INSTRUCTION after _exit (if the syscall does not terminate the process, the system is broken). Checked on x86_64-linux-gnu and aarch64-linux-gnu. --- include/bits/unistd_ext.h | 3 + include/stdlib.h | 6 + manual/startup.texi | 5 +- nptl/pthread_kill.c | 11 ++ posix/fork.c | 2 + signal/sigaction.c | 15 ++- stdlib/abort.c | 131 ++++++++------------- sysdeps/generic/internal-signals.h | 27 ++++- sysdeps/generic/internal-sigset.h | 26 ++++ sysdeps/htl/pthreadP.h | 2 + sysdeps/nptl/_Fork.c | 9 ++ sysdeps/nptl/pthreadP.h | 1 + sysdeps/unix/sysv/linux/internal-signals.h | 9 ++ sysdeps/unix/sysv/linux/internal-sigset.h | 2 +- sysdeps/unix/sysv/linux/spawni.c | 8 +- 15 files changed, 163 insertions(+), 94 deletions(-) create mode 100644 sysdeps/generic/internal-sigset.h diff --git a/include/bits/unistd_ext.h b/include/bits/unistd_ext.h index 277be05746..eeb07baf70 100644 --- a/include/bits/unistd_ext.h +++ b/include/bits/unistd_ext.h @@ -3,4 +3,7 @@ #ifndef _ISOMAC extern int __close_range (unsigned int lowfd, unsigned int highfd, int flags); libc_hidden_proto (__close_range); + +extern pid_t __gettid (void); +libc_hidden_proto (__gettid); #endif diff --git a/include/stdlib.h b/include/stdlib.h index 0cab3f5b56..07f074bcce 100644 --- a/include/stdlib.h +++ b/include/stdlib.h @@ -20,6 +20,7 @@ # include # include +# include extern __typeof (strtol_l) __strtol_l; extern __typeof (strtoul_l) __strtoul_l; @@ -77,6 +78,11 @@ libc_hidden_proto (__isoc23_strtoull_l) # define strtoull_l __isoc23_strtoull_l #endif +extern void __abort_fork_reset_child (void) attribute_hidden; +extern void __abort_lock_lock (internal_sigset_t *set) attribute_hidden; +extern void __abort_lock_unlock (const internal_sigset_t *set) + attribute_hidden; + libc_hidden_proto (exit) libc_hidden_proto (abort) libc_hidden_proto (getenv) diff --git a/manual/startup.texi b/manual/startup.texi index 747beed4d9..139e359258 100644 --- a/manual/startup.texi +++ b/manual/startup.texi @@ -1010,10 +1010,7 @@ for this function is in @file{stdlib.h}. @deftypefun void abort (void) @standards{ISO, stdlib.h} -@safety{@prelim{}@mtsafe{}@asunsafe{@asucorrupt{}}@acunsafe{@aculock{} @acucorrupt{}}} -@c The implementation takes a recursive lock and attempts to support -@c calls from signal handlers, but if we're in the middle of flushing or -@c using streams, we may encounter them in inconsistent states. +@safety{@prelim{}@mtsafe{}@asafe{}@acusafe{} The @code{abort} function causes abnormal program termination. This does not execute cleanup functions registered with @code{atexit} or @code{on_exit}. diff --git a/nptl/pthread_kill.c b/nptl/pthread_kill.c index 71e5a7bf5b..fa5121a583 100644 --- a/nptl/pthread_kill.c +++ b/nptl/pthread_kill.c @@ -69,6 +69,17 @@ __pthread_kill_implementation (pthread_t threadid, int signo, int no_tid) return ret; } +/* Send the signal SIGNO to the caller. Used by abort and called where the + signals are being already blocked and there is no need to synchronize with + exit_lock. */ +int +__pthread_raise_internal (int signo) +{ + /* Use the gettid syscall so it works after vfork. */ + int ret = INTERNAL_SYSCALL_CALL (tgkill, __getpid (), __gettid(), signo); + return INTERNAL_SYSCALL_ERROR_P (ret) ? INTERNAL_SYSCALL_ERRNO (ret) : 0; +} + int __pthread_kill_internal (pthread_t threadid, int signo) { diff --git a/posix/fork.c b/posix/fork.c index 298765a1ff..c2b476ff2d 100644 --- a/posix/fork.c +++ b/posix/fork.c @@ -84,6 +84,8 @@ __libc_fork (void) fork_system_setup_after_fork (); + call_function_static_weak (__abort_fork_reset_child); + /* Release malloc locks. */ call_function_static_weak (__malloc_fork_unlock_child); diff --git a/signal/sigaction.c b/signal/sigaction.c index 811062ae96..c2931b8b04 100644 --- a/signal/sigaction.c +++ b/signal/sigaction.c @@ -16,8 +16,9 @@ . */ #include -#include #include +#include +#include /* If ACT is not NULL, change the action for SIG to *ACT. If OACT is not NULL, put the old action for SIG in *OACT. */ @@ -30,7 +31,17 @@ __sigaction (int sig, const struct sigaction *act, struct sigaction *oact) return -1; } - return __libc_sigaction (sig, act, oact); + internal_sigset_t set; + + if (sig == SIGABRT) + __abort_lock_lock (&set); + + int r = __libc_sigaction (sig, act, oact); + + if (sig == SIGABRT) + __abort_lock_unlock (&set); + + return r; } libc_hidden_def (__sigaction) weak_alias (__sigaction, sigaction) diff --git a/stdlib/abort.c b/stdlib/abort.c index e2b84baac4..0ebebc55d1 100644 --- a/stdlib/abort.c +++ b/stdlib/abort.c @@ -15,13 +15,11 @@ License along with the GNU C Library; if not, see . */ -#include #include -#include -#include -#include -#include #include +#include +#include +#include /* Try to get a machine dependent instruction which will make the program crash. This is used in case everything else fails. */ @@ -35,89 +33,56 @@ struct abort_msg_s *__abort_msg; libc_hidden_def (__abort_msg) -/* We must avoid to run in circles. Therefore we remember how far we - already got. */ -static int stage; +/* The lock is used to prevent multiple thread to change the SIGABRT + to SIG_IGN while abort tries to change to SIG_DFL, and to avoid + a new process to see a wrong disposition if there is a SIGABRT + handler installed. */ +__libc_lock_define_initialized (static, lock); + +void +__abort_fork_reset_child (void) +{ + __libc_lock_init (lock); +} -/* We should be prepared for multiple threads trying to run abort. */ -__libc_lock_define_initialized_recursive (static, lock); +void +__abort_lock_lock (internal_sigset_t *set) +{ + internal_signal_block_all (set); + __libc_lock_lock (lock); +} +void +__abort_lock_unlock (const internal_sigset_t *set) +{ + __libc_lock_unlock (lock); + internal_signal_restore_set (set); +} /* Cause an abnormal program termination with core-dump. */ -void +_Noreturn void abort (void) { - struct sigaction act; - - /* First acquire the lock. */ - __libc_lock_lock_recursive (lock); - - /* Now it's for sure we are alone. But recursive calls are possible. */ - - /* Unblock SIGABRT. */ - if (stage == 0) - { - ++stage; - internal_sigset_t sigs; - internal_sigemptyset (&sigs); - internal_sigaddset (&sigs, SIGABRT); - internal_sigprocmask (SIG_UNBLOCK, &sigs, NULL); - } - - /* Send signal which possibly calls a user handler. */ - if (stage == 1) - { - /* This stage is special: we must allow repeated calls of - `abort' when a user defined handler for SIGABRT is installed. - This is risky since the `raise' implementation might also - fail but I don't see another possibility. */ - int save_stage = stage; - - stage = 0; - __libc_lock_unlock_recursive (lock); - - raise (SIGABRT); - - __libc_lock_lock_recursive (lock); - stage = save_stage + 1; - } - - /* There was a handler installed. Now remove it. */ - if (stage == 2) - { - ++stage; - memset (&act, '\0', sizeof (struct sigaction)); - act.sa_handler = SIG_DFL; - __sigfillset (&act.sa_mask); - act.sa_flags = 0; - __sigaction (SIGABRT, &act, NULL); - } - - /* Try again. */ - if (stage == 3) - { - ++stage; - raise (SIGABRT); - } - - /* Now try to abort using the system specific command. */ - if (stage == 4) - { - ++stage; - ABORT_INSTRUCTION; - } - - /* If we can't signal ourselves and the abort instruction failed, exit. */ - if (stage == 5) - { - ++stage; - _exit (127); - } - - /* If even this fails try to use the provided instruction to crash - or otherwise make sure we never return. */ - while (1) - /* Try for ever and ever. */ - ABORT_INSTRUCTION; + raise (SIGABRT); + + /* There is a SIGABRT handle installed and it returned, or SIGABRT was + blocked or ignored. In this case use a AS-safe lock to prevent sigaction + to change the signal dispositioni (it will block on __abort_lock), + reinstall the handle to abort the process, and re-raise the signal. + + The handle can not change again because sigaction blocks on the lock. */ + __abort_lock_lock (NULL); + + struct sigaction act = {.sa_handler = SIG_DFL, .sa_flags = 0 }; + __sigfillset (&act.sa_mask); + __libc_sigaction (SIGABRT, &act, NULL); + __pthread_raise_internal (SIGABRT); + internal_signal_unblock_signal (SIGABRT); + + /* This code should be unreachable, try the arch-specific code and the + syscall fallback. */ + ABORT_INSTRUCTION; + + _exit (127); } libc_hidden_def (abort) diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h index 3db100be10..e031a96bac 100644 --- a/sysdeps/generic/internal-signals.h +++ b/sysdeps/generic/internal-signals.h @@ -20,6 +20,7 @@ # define __INTERNAL_SIGNALS_H #include +#include #include #include #include @@ -39,10 +40,32 @@ clear_internal_signals (sigset_t *set) { } -typedef sigset_t internal_sigset_t; - #define internal_sigemptyset(__s) __sigemptyset (__s) +#define internal_sigfillset(__s) __sigfillset (__s) #define internal_sigaddset(__s, __i) __sigaddset (__s, __i) #define internal_sigprocmask(__h, __s, __o) __sigprocmask (__h, __s, __o) +static inline void +internal_signal_block_all (internal_sigset_t *oset) +{ + internal_sigset_t set; + internal_sigfillset (&set); + internal_sigprocmask (SIG_BLOCK, &set, oset); +} + +static inline void +internal_signal_restore_set (const internal_sigset_t *set) +{ + internal_sigprocmask (SIG_SETMASK, set, NULL); +} + +static inline void +internal_signal_unblock_signal (int sig) +{ + internal_sigset_t set; + internal_sigemptyset (&set); + internal_sigaddset (&set, sig); + internal_sigprocmask (SIG_UNBLOCK, &set, NULL); +} + #endif /* __INTERNAL_SIGNALS_H */ diff --git a/sysdeps/generic/internal-sigset.h b/sysdeps/generic/internal-sigset.h new file mode 100644 index 0000000000..80279ffc47 --- /dev/null +++ b/sysdeps/generic/internal-sigset.h @@ -0,0 +1,26 @@ +/* Internal sigset_t definition. + Copyright (C) 2022-2023 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#ifndef _INTERNAL_SIGSET_H +#define _INTERNAL_SIGSET_H + +#include + +typedef sigset_t internal_sigset_t; + +#endif diff --git a/sysdeps/htl/pthreadP.h b/sysdeps/htl/pthreadP.h index cf8a2efe86..b0c9ceb23a 100644 --- a/sysdeps/htl/pthreadP.h +++ b/sysdeps/htl/pthreadP.h @@ -92,6 +92,8 @@ int __pthread_attr_setstack (pthread_attr_t *__attr, void *__stackaddr, int __pthread_attr_getstack (const pthread_attr_t *, void **, size_t *); void __pthread_testcancel (void); +#define __pthread_raise_internal(__sig) raise (__sig) + libc_hidden_proto (__pthread_self) #if IS_IN (libpthread) diff --git a/sysdeps/nptl/_Fork.c b/sysdeps/nptl/_Fork.c index ef199ddbc3..7d970e3978 100644 --- a/sysdeps/nptl/_Fork.c +++ b/sysdeps/nptl/_Fork.c @@ -17,11 +17,17 @@ . */ #include +#include #include pid_t _Fork (void) { + /* The lock acquisition needs to be AS-safe to avoid deadlock if _Fork is + called from the signal handler that has interrupted fork itself. */ + internal_sigset_t set; + __abort_lock_lock (&set); + pid_t pid = arch_fork (&THREAD_SELF->tid); if (pid == 0) { @@ -44,6 +50,9 @@ _Fork (void) INTERNAL_SYSCALL_CALL (set_robust_list, &self->robust_head, sizeof (struct robust_list_head)); } + + __abort_lock_unlock (&set); + return pid; } libc_hidden_def (_Fork) diff --git a/sysdeps/nptl/pthreadP.h b/sysdeps/nptl/pthreadP.h index 30e8a2d177..b8ed954e5a 100644 --- a/sysdeps/nptl/pthreadP.h +++ b/sysdeps/nptl/pthreadP.h @@ -508,6 +508,7 @@ libc_hidden_proto (__pthread_kill) extern int __pthread_cancel (pthread_t th); extern int __pthread_kill_internal (pthread_t threadid, int signo) attribute_hidden; +extern int __pthread_raise_internal (int signo) attribute_hidden; extern void __pthread_exit (void *value) __attribute__ ((__noreturn__)); libc_hidden_proto (__pthread_exit) extern int __pthread_join (pthread_t threadid, void **thread_return); diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h index a6fae59aaa..6e3a3d7692 100644 --- a/sysdeps/unix/sysv/linux/internal-signals.h +++ b/sysdeps/unix/sysv/linux/internal-signals.h @@ -90,6 +90,15 @@ internal_signal_restore_set (const internal_sigset_t *set) __NSIG_BYTES); } +static inline void +internal_signal_unblock_signal (int sig) +{ + internal_sigset_t set; + internal_sigemptyset (&set); + internal_sigaddset (&set, sig); + INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_UNBLOCK, &set, NULL, + __NSIG_BYTES); +} /* It is used on timer_create code directly on sigwaitinfo call, so it can not use the internal_sigset_t definitions. */ diff --git a/sysdeps/unix/sysv/linux/internal-sigset.h b/sysdeps/unix/sysv/linux/internal-sigset.h index 5d7020b42d..4b19affd75 100644 --- a/sysdeps/unix/sysv/linux/internal-sigset.h +++ b/sysdeps/unix/sysv/linux/internal-sigset.h @@ -21,7 +21,7 @@ #include -typedef struct +typedef struct _internal_sigset_t { unsigned long int __val[__NSIG_WORDS]; } internal_sigset_t; diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c index f57e92815e..be72fa716f 100644 --- a/sysdeps/unix/sysv/linux/spawni.c +++ b/sysdeps/unix/sysv/linux/spawni.c @@ -383,7 +383,11 @@ __spawnix (int *pid, const char *file, args.pidfd = 0; args.xflags = xflags; - internal_signal_block_all (&args.oldmask); + /* Avoid the potential issues is if caller sets a SIG_IGN for SIGABRT, calls + abort, and another thread issues posix_spawn just after the sigaction + returns. With default options (not setting POSIX_SPAWN_SETSIGDEF), the + process can still see SIG_DFL for SIGABRT, where it should be SIG_IGN. */ + __abort_lock_lock (&args.oldmask); /* The clone flags used will create a new child that will run in the same memory space (CLONE_VM) and the execution of calling thread will be @@ -474,7 +478,7 @@ __spawnix (int *pid, const char *file, if ((ec == 0) && (pid != NULL)) *pid = use_pidfd ? args.pidfd : new_pid; - internal_signal_restore_set (&args.oldmask); + __abort_lock_unlock (&args.oldmask); __pthread_setcancelstate (state, NULL);