From patchwork Fri Feb 24 17:31:10 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adam Yi X-Patchwork-Id: 65608 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 42519385B53E for ; Fri, 24 Feb 2023 17:31:49 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 42519385B53E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1677259909; bh=Na9EGqW2Mb3wOnJ5GUHrTzZde8ZOfvsXZN86c1vw6Cw=; h=Date:Subject:To:Cc:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=BvrE2YIeA/b9KXgK4vUiKuSrXdtjjl+XpRjCo7Bn3tL82L4MDRdHyP/aIIdragTf6 sm9Pa5oJ/2FqGRzPduTSNZ/Dy34FwbM2zR/mko4G32t74035kdfPEAV+uO9kd9dSO4 lc1Fj49GkrxdvaUlyJgncIRVieRAfPA9UguGbHXo= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mxout5.mail.janestreet.com (mxout5.mail.janestreet.com [64.215.233.18]) by sourceware.org (Postfix) with ESMTPS id 246D0385840F for ; Fri, 24 Feb 2023 17:31:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 246D0385840F Received: from mail-qk1-f200.google.com ([209.85.222.200]) by mxgoog2.mail.janestreet.com with esmtps (TLS1.3:TLS_AES_128_GCM_SHA256:128) (Exim 4.96) id 1pVbup-0026w9-1E for libc-alpha@sourceware.org; Fri, 24 Feb 2023 12:31:27 -0500 Received: by mail-qk1-f200.google.com with SMTP id c15-20020a37e10f000000b00741a3333881so7036384qkm.13 for ; Fri, 24 Feb 2023 09:31:27 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=Na9EGqW2Mb3wOnJ5GUHrTzZde8ZOfvsXZN86c1vw6Cw=; b=Y4ccenjSVW0b2JyX8HPgPWGnKwtFtpF8FL90R6JH9+c2wybrzkXiEmu6fCEzlVquFy AG9sBpZdYFLVnIomsq6wK4qhpBtgmRPVKhPz7O5HmmWNLSle4hwjezs0mOqKp7G9key0 WLxJg7L1KiuHH76V9lmnPjMzqlVc7R5EB0PxT5ti13mOWErRt13suZPir5RDTrpsXAKq jhB5YHaKEpgJaBgVSbKno9Dm4ON8Ovn0AjyeBShry3zfAJArQWVTc4D3vA9h5BSWVbfR rbdULzg7KNpckFFHFvW+cL6lnyFMB01KPLssBpIZh9HfiiGOJbCb/0VYe8SFB1fNXadc nTmw== X-Gm-Message-State: AO0yUKUrwJ6eo9ZAECEtI+lQkjP1sjBKQKWek7EQEwm04oIYGANIyiqV 3+RYx2VjH7Ni/pP3QdSX8GUUqXE3pkgqgxKkUGSbREax8FL4mII1MQewhuFK+dYIwnpMIb+zYfq UsR6PwMw4Qo52DuWzt+3pehT/nh+l404rsd8j2i3QovzZZA== X-Received: by 2002:ac8:561b:0:b0:3bf:b9d9:6759 with SMTP id 27-20020ac8561b000000b003bfb9d96759mr864134qtr.8.1677259886949; Fri, 24 Feb 2023 09:31:26 -0800 (PST) X-Google-Smtp-Source: AK7set+ThMx1DIH5lUW5v7AFx38Inw/WsDYuiz5qM8Tfu45bo9mvNmCkw/vrg26R5BLS4TDz6jmyT5dpHsfhw0Hx/G0= X-Received: by 2002:ac8:561b:0:b0:3bf:b9d9:6759 with SMTP id 27-20020ac8561b000000b003bfb9d96759mr864130qtr.8.1677259886581; Fri, 24 Feb 2023 09:31:26 -0800 (PST) MIME-Version: 1.0 Date: Sat, 25 Feb 2023 01:31:10 +0800 Message-ID: Subject: [PATCH] posix: Fix system blocks SIGCHLD erroneously [BZ #30163] To: libc-alpha@sourceware.org Cc: adhemerval.zanella@linaro.org, Adam Yi X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_PASS, 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.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Adam Yi via Libc-alpha From: Adam Yi Reply-To: Adam Yi Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" Fix bug that SIGCHLD is erroneously blocked forever in the following scenario: 1. Thread A calls system but hasn't returned yet 2. Thread B calls another system but returns SIGCHLD would be blocked forever in thread B after its system() returns, even after the system() in thread A returns. Although POSIX does not require, glibc system implementation aims to be thread and cancellation safe. This bug was introduced in 5fb7fc96350575c9adb1316833e48ca11553be49 when we moved reverting signal mask to happen when the last concurrently running system returns, despite that signal mask is per thread. This commit reverts this logic and adds a test. --- stdlib/tst-system.c | 27 +++++++++++++++++++++++++++ support/shell-container.c | 10 ++++++++++ sysdeps/posix/system.c | 6 +++--- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/stdlib/tst-system.c b/stdlib/tst-system.c index 634acfe264..be95c2de44 100644 --- a/stdlib/tst-system.c +++ b/stdlib/tst-system.c @@ -20,6 +20,8 @@ #include #include #include +#include +#include #include #include @@ -71,6 +73,19 @@ call_system (void *closure) } } +static void * +sleep_and_check_sigchld (void *seconds) +{ + char cmd[namemax]; + sprintf(cmd, "sleep %" PRIdPTR, (intptr_t) seconds); + TEST_COMPARE (system (cmd), 0); + + sigset_t blocked = {0}; + TEST_COMPARE (sigprocmask (SIG_BLOCK, NULL, &blocked), 0); + TEST_COMPARE (sigismember (&blocked, SIGCHLD), 0); + return NULL; +} + static int do_test (void) { @@ -154,6 +169,18 @@ do_test (void) xchmod (_PATH_BSHELL, st.st_mode); } + { + pthread_t long_sleep_thread, short_sleep_thread; + + TEST_COMPARE (pthread_create (&long_sleep_thread, NULL, + sleep_and_check_sigchld, (void *) 2), 0); + TEST_COMPARE (pthread_create (&short_sleep_thread, NULL, + sleep_and_check_sigchld, (void *) 1), 0); + + TEST_COMPARE (pthread_join (short_sleep_thread, NULL), 0); + TEST_COMPARE (pthread_join (long_sleep_thread, NULL), 0); + } + TEST_COMPARE (system (""), 0); return 0; diff --git a/support/shell-container.c b/support/shell-container.c index e9ac9b6d04..083426550b 100644 --- a/support/shell-container.c +++ b/support/shell-container.c @@ -171,6 +171,15 @@ kill_func (char **argv) return 0; } +/* Emulate the "/bin/sleep" command. Options are ignored. */ +static int +sleep_func (char **argv) +{ + int secs = atoi (argv[0]); + sleep (secs); + return 0; +} + /* This is a list of all the built-in commands we understand. */ static struct { const char *name; @@ -181,6 +190,7 @@ static struct { { "cp", copy_func }, { "exit", exit_func }, { "kill", kill_func }, + { "sleep", sleep_func }, { NULL, NULL } }; diff --git a/sysdeps/posix/system.c b/sysdeps/posix/system.c index 2335a99184..d77720a625 100644 --- a/sysdeps/posix/system.c +++ b/sysdeps/posix/system.c @@ -179,16 +179,16 @@ do_system (const char *line) as if the shell had terminated using _exit(127). */ status = W_EXITCODE (127, 0); + /* sigaction can not fail with SIGINT/SIGQUIT used with old + disposition. Same applies for sigprocmask. */ DO_LOCK (); if (SUB_REF () == 0) { - /* sigaction can not fail with SIGINT/SIGQUIT used with old - disposition. Same applies for sigprocmask. */ __sigaction (SIGINT, &intr, NULL); __sigaction (SIGQUIT, &quit, NULL); - __sigprocmask (SIG_SETMASK, &omask, NULL); } DO_UNLOCK (); + __sigprocmask (SIG_SETMASK, &omask, NULL); if (ret != 0) __set_errno (ret);