diff options
author | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2023-07-31 12:44:37 -0300 |
---|---|---|
committer | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2024-06-19 09:05:43 -0300 |
commit | 80f34f5173a29833c4f9e73a9725376448769a99 (patch) | |
tree | 4f14d07fe9a9d5c677818d23bda45f9288a5b02d /sysdeps/unix | |
parent | c49e66c7e507f2d37c4725ce4680f19179cfa44e (diff) | |
download | glibc-azanella/bz26275-abort-as.zip glibc-azanella/bz26275-abort-as.tar.gz glibc-azanella/bz26275-abort-as.tar.bz2 |
stdlib: Make abort AS-safe (BZ 26275)azanella/bz26275-abort-as
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 is issues 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 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 nned to use a loop of
ABORT_INSTRUCTION after _exit (if the syscall does not terminate the
process, the system is really broken).
Checked on x86_64-linux-gnu and aarch64-linux-gnu.
Diffstat (limited to 'sysdeps/unix')
-rw-r--r-- | sysdeps/unix/sysv/linux/internal-signals.h | 9 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/internal-sigset.h | 2 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/spawni.c | 6 |
3 files changed, 14 insertions, 3 deletions
diff --git a/sysdeps/unix/sysv/linux/internal-signals.h b/sysdeps/unix/sysv/linux/internal-signals.h index a6fae59..6e3a3d7 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 5d7020b..4b19aff 100644 --- a/sysdeps/unix/sysv/linux/internal-sigset.h +++ b/sysdeps/unix/sysv/linux/internal-sigset.h @@ -21,7 +21,7 @@ #include <sigsetops.h> -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 e8ed2ba..5b53924 100644 --- a/sysdeps/unix/sysv/linux/spawni.c +++ b/sysdeps/unix/sysv/linux/spawni.c @@ -383,7 +383,9 @@ __spawnix (int *pid, const char *file, args.pidfd = 0; args.xflags = xflags; - internal_signal_block_all (&args.oldmask); + /* Avoid the abort to change the SIGABRT disposition to SIG_DFL for the + case POSIX_SPAWN_SETSIGDEF is not set and SIG_IGN is current handle. */ + __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 @@ -465,7 +467,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); |