aboutsummaryrefslogtreecommitdiff
path: root/sysdeps
diff options
context:
space:
mode:
authorAdhemerval Zanella <adhemerval.zanella@linaro.org>2024-10-03 15:41:10 -0300
committerAdhemerval Zanella <adhemerval.zanella@linaro.org>2024-10-08 14:40:12 -0300
commitd40ac01cbbc66e6d9dbd8e3485605c63b2178251 (patch)
treed845257209902e0392df53491dcca8f7b9f64eaf /sysdeps
parent55d33108c72c9b3e231b83a55e16a9ba1190c768 (diff)
downloadglibc-d40ac01cbbc66e6d9dbd8e3485605c63b2178251.zip
glibc-d40ac01cbbc66e6d9dbd8e3485605c63b2178251.tar.gz
glibc-d40ac01cbbc66e6d9dbd8e3485605c63b2178251.tar.bz2
stdlib: Make abort/_Exit AS-safe (BZ 26275)
The recursive lock used on abort does not synchronize with a 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 signal, might create programs with a non-expected signal mask. With the default option (without POSIX_SPAWN_SETSIGDEF), the process can see SIG_DFL for SIGABRT, where it should be SIG_IGN. To fix the AS-safe, raise() does not change the process signal mask, and an AS-safe lock is used if a SIGABRT is installed or the process is blocked or ignored. With the signal mask change removal, there is no need to use a recursive loc. The lock is also taken on both _Fork() and posix_spawn(), to avoid the spawn process to see the abort handler as SIG_DFL. A read-write lock is used to avoid serialize _Fork and posix_spawn execution. Both sigaction (SIGABRT) and abort() requires to lock as writer (since both change the disposition). 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). The proposed fix changes how setjmp works on a SIGABRT handler, where glibc does not save the signal mask. So usage like the below will now always abort. static volatile int chk_fail_ok; static jmp_buf chk_fail_buf; static void handler (int sig) { if (chk_fail_ok) { chk_fail_ok = 0; longjmp (chk_fail_buf, 1); } else _exit (127); } [...] signal (SIGABRT, handler); [....] chk_fail_ok = 1; if (! setjmp (chk_fail_buf)) { // Something that can calls abort, like a failed fortify function. chk_fail_ok = 0; printf ("FAIL\n"); } Such cases will need to use sigsetjmp instead. The _dl_start_profile calls sigaction through _profil, and to avoid pulling abort() on loader the call is replaced with __libc_sigaction. Checked on x86_64-linux-gnu and aarch64-linux-gnu. Reviewed-by: DJ Delorie <dj@redhat.com>
Diffstat (limited to 'sysdeps')
-rw-r--r--sysdeps/generic/internal-signals.h27
-rw-r--r--sysdeps/generic/internal-sigset.h26
-rw-r--r--sysdeps/htl/pthreadP.h2
-rw-r--r--sysdeps/nptl/_Fork.c8
-rw-r--r--sysdeps/nptl/pthreadP.h1
-rw-r--r--sysdeps/posix/profil.c6
-rw-r--r--sysdeps/unix/sysv/linux/internal-signals.h9
-rw-r--r--sysdeps/unix/sysv/linux/internal-sigset.h2
-rw-r--r--sysdeps/unix/sysv/linux/spawni.c8
9 files changed, 78 insertions, 11 deletions
diff --git a/sysdeps/generic/internal-signals.h b/sysdeps/generic/internal-signals.h
index 3db100b..e031a96 100644
--- a/sysdeps/generic/internal-signals.h
+++ b/sysdeps/generic/internal-signals.h
@@ -20,6 +20,7 @@
# define __INTERNAL_SIGNALS_H
#include <signal.h>
+#include <internal-sigset.h>
#include <sigsetops.h>
#include <stdbool.h>
#include <stddef.h>
@@ -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 0000000..80279ff
--- /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
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _INTERNAL_SIGSET_H
+#define _INTERNAL_SIGSET_H
+
+#include <signal.h>
+
+typedef sigset_t internal_sigset_t;
+
+#endif
diff --git a/sysdeps/htl/pthreadP.h b/sysdeps/htl/pthreadP.h
index cf8a2ef..b0c9ceb 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 fd2cfc7..52c90e6 100644
--- a/sysdeps/nptl/_Fork.c
+++ b/sysdeps/nptl/_Fork.c
@@ -17,15 +17,17 @@
<https://www.gnu.org/licenses/>. */
#include <arch-fork.h>
+#include <libc-lock.h>
#include <pthreadP.h>
pid_t
_Fork (void)
{
/* Block all signals to avoid revealing the inconsistent TCB state
- to a signal handler after fork. */
+ to a signal handler after fork. The abort lock should AS-safe
+ to avoid deadlock if _Fork is called from a signal handler. */
internal_sigset_t original_sigmask;
- internal_signal_block_all (&original_sigmask);
+ __abort_lock_rdlock (&original_sigmask);
pid_t pid = arch_fork (&THREAD_SELF->tid);
if (pid == 0)
@@ -50,7 +52,7 @@ _Fork (void)
sizeof (struct robust_list_head));
}
- internal_signal_restore_set (&original_sigmask);
+ __abort_lock_unlock (&original_sigmask);
return pid;
}
libc_hidden_def (_Fork)
diff --git a/sysdeps/nptl/pthreadP.h b/sysdeps/nptl/pthreadP.h
index 7d9b95e..c2db165 100644
--- a/sysdeps/nptl/pthreadP.h
+++ b/sysdeps/nptl/pthreadP.h
@@ -517,6 +517,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/posix/profil.c b/sysdeps/posix/profil.c
index 3b3c87e..6f4aa00 100644
--- a/sysdeps/posix/profil.c
+++ b/sysdeps/posix/profil.c
@@ -82,7 +82,7 @@ __profil (u_short *sample_buffer, size_t size, size_t offset, u_int scale)
if (__setitimer (ITIMER_PROF, &otimer, NULL) < 0)
return -1;
samples = NULL;
- return __sigaction (SIGPROF, &oact, NULL);
+ return __libc_sigaction (SIGPROF, &oact, NULL);
}
if (samples)
@@ -90,7 +90,7 @@ __profil (u_short *sample_buffer, size_t size, size_t offset, u_int scale)
/* Was already turned on. Restore old timer and signal handler
first. */
if (__setitimer (ITIMER_PROF, &otimer, NULL) < 0
- || __sigaction (SIGPROF, &oact, NULL) < 0)
+ || __libc_sigaction (SIGPROF, &oact, NULL) < 0)
return -1;
}
#else
@@ -114,7 +114,7 @@ __profil (u_short *sample_buffer, size_t size, size_t offset, u_int scale)
#endif
act.sa_flags |= SA_RESTART;
__sigfillset (&act.sa_mask);
- if (__sigaction (SIGPROF, &act, oact_ptr) < 0)
+ if (__libc_sigaction (SIGPROF, &act, oact_ptr) < 0)
return -1;
timer.it_value.tv_sec = 0;
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 f57e928..1edf62c 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 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_rdlock (&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);