posix: Fix system blocks SIGCHLD erroneously [BZ #30163]

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
5fb7fc9635 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.

Signed-off-by: Adam Yi <ayi@janestreet.com>
Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
(cherry picked from commit 436a604b7d)
This commit is contained in:
Adam Yi 2023-03-07 07:30:02 -05:00 committed by Florian Weimer
parent 9cc2f41e52
commit 78ab913eab
8 changed files with 141 additions and 3 deletions

1
NEWS
View File

@ -22,6 +22,7 @@ The following bugs are resolved with this release:
[30125] dynamic-link: [regression, bisected] glibc-2.37 creates new
symlink for libraries without soname
[30151] gshadow: Matching sgetsgent, sgetsgent_r ERANGE handling
[30163] posix: Fix system blocks SIGCHLD erroneously
[30305] x86_64: Fix asm constraints in feraiseexcept
Version 2.37

View File

@ -25,6 +25,7 @@
#include <support/check.h>
#include <support/temp_file.h>
#include <support/support.h>
#include <support/xthread.h>
#include <support/xunistd.h>
static char *tmpdir;
@ -71,6 +72,20 @@ call_system (void *closure)
}
}
static void *
sleep_and_check_sigchld (void *closure)
{
double *seconds = (double *) closure;
char cmd[namemax];
sprintf (cmd, "sleep %lf" , *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,17 @@ do_test (void)
xchmod (_PATH_BSHELL, st.st_mode);
}
{
pthread_t long_sleep_thread = xpthread_create (NULL,
sleep_and_check_sigchld,
&(double) { 0.2 });
pthread_t short_sleep_thread = xpthread_create (NULL,
sleep_and_check_sigchld,
&(double) { 0.1 });
xpthread_join (short_sleep_thread);
xpthread_join (long_sleep_thread);
}
TEST_COMPARE (system (""), 0);
return 0;

View File

@ -32,6 +32,8 @@ libsupport-routines = \
check_hostent \
check_netent \
delayed_exit \
dtotimespec \
dtotimespec-time64 \
ignore_stderr \
next_to_fault \
oom_error \

View File

@ -0,0 +1,27 @@
/* Convert double to timespec. 64-bit time support.
Copyright (C) 2011-2023 Free Software Foundation, Inc.
This file is part of the GNU C Library and is also part of gnulib.
Patches to this file should be submitted to both projects.
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
<https://www.gnu.org/licenses/>. */
#include <time.h>
#if __TIMESIZE != 64
# define timespec __timespec64
# define time_t __time64_t
# define dtotimespec dtotimespec_time64
# include "dtotimespec.c"
#endif

50
support/dtotimespec.c Normal file
View File

@ -0,0 +1,50 @@
/* Convert double to timespec.
Copyright (C) 2011-2023 Free Software Foundation, Inc.
This file is part of the GNU C Library and is also part of gnulib.
Patches to this file should be submitted to both projects.
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
<https://www.gnu.org/licenses/>. */
/* Convert the double value SEC to a struct timespec. Round toward
positive infinity. On overflow, return an extremal value. */
#include <support/timespec.h>
#include <intprops.h>
struct timespec
dtotimespec (double sec)
{
if (sec <= TYPE_MINIMUM (time_t))
return make_timespec (TYPE_MINIMUM (time_t), 0);
else if (sec >= 1.0 + TYPE_MAXIMUM (time_t))
return make_timespec (TYPE_MAXIMUM (time_t), TIMESPEC_HZ - 1);
else
{
time_t s = sec;
double frac = TIMESPEC_HZ * (sec - s);
long ns = frac;
ns += ns < frac;
s += ns / TIMESPEC_HZ;
ns %= TIMESPEC_HZ;
if (ns < 0)
{
s--;
ns += TIMESPEC_HZ;
}
return make_timespec (s, ns);
}
}

View File

@ -39,6 +39,7 @@
#include <error.h>
#include <support/support.h>
#include <support/timespec.h>
/* Design considerations
@ -171,6 +172,32 @@ kill_func (char **argv)
return 0;
}
/* Emulate the "/bin/sleep" command. No suffix support. Options are
ignored. */
static int
sleep_func (char **argv)
{
if (argv[0] == NULL)
{
fprintf (stderr, "sleep: missing operand\n");
return 1;
}
char *endptr = NULL;
double sec = strtod (argv[0], &endptr);
if (endptr == argv[0] || errno == ERANGE || sec < 0)
{
fprintf (stderr, "sleep: invalid time interval '%s'\n", argv[0]);
return 1;
}
struct timespec ts = dtotimespec (sec);
if (nanosleep (&ts, NULL) < 0)
{
fprintf (stderr, "sleep: failed to nanosleep: %s\n", strerror (errno));
return 1;
}
return 0;
}
/* This is a list of all the built-in commands we understand. */
static struct {
const char *name;
@ -181,6 +208,7 @@ static struct {
{ "cp", copy_func },
{ "exit", exit_func },
{ "kill", kill_func },
{ "sleep", sleep_func },
{ NULL, NULL }
};

View File

@ -57,6 +57,8 @@ int support_timespec_check_in_range (struct timespec expected,
struct timespec observed,
double lower_bound, double upper_bound);
struct timespec dtotimespec (double sec) __attribute__((const));
#else
struct timespec __REDIRECT (timespec_add, (struct timespec, struct timespec),
timespec_add_time64);
@ -82,6 +84,8 @@ int __REDIRECT (support_timespec_check_in_range, (struct timespec expected,
double lower_bound,
double upper_bound),
support_timespec_check_in_range_time64);
struct timespec __REDIRECT (dtotimespec, (double sec), dtotimespec_time64);
#endif
/* Check that the timespec on the left represents a time before the

View File

@ -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);