aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAdhemerval Zanella <adhemerval.zanella@linaro.org>2023-07-31 12:44:37 -0300
committerAdhemerval Zanella <adhemerval.zanella@linaro.org>2024-06-19 09:05:43 -0300
commit80f34f5173a29833c4f9e73a9725376448769a99 (patch)
tree4f14d07fe9a9d5c677818d23bda45f9288a5b02d
parentc49e66c7e507f2d37c4725ce4680f19179cfa44e (diff)
downloadglibc-80f34f5173a29833c4f9e73a9725376448769a99.zip
glibc-80f34f5173a29833c4f9e73a9725376448769a99.tar.gz
glibc-80f34f5173a29833c4f9e73a9725376448769a99.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.
-rw-r--r--include/bits/unistd_ext.h3
-rw-r--r--include/stdlib.h6
-rw-r--r--manual/startup.texi3
-rw-r--r--nptl/pthread_kill.c11
-rw-r--r--posix/fork.c2
-rw-r--r--signal/sigaction.c15
-rw-r--r--stdlib/abort.c121
-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.c9
-rw-r--r--sysdeps/nptl/pthreadP.h1
-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.c6
15 files changed, 154 insertions, 89 deletions
diff --git a/include/bits/unistd_ext.h b/include/bits/unistd_ext.h
index 277be05..eeb07ba 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 0cab3f5..07f074b 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -20,6 +20,7 @@
# include <sys/stat.h>
# include <rtld-malloc.h>
+# include <internal-sigset.h>
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 224dd98..145acf0 100644
--- a/manual/startup.texi
+++ b/manual/startup.texi
@@ -991,9 +991,6 @@ 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.
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 71e5a7b..fa5121a 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 298765a..c2b476f 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 811062a..c2931b8 100644
--- a/signal/sigaction.c
+++ b/signal/sigaction.c
@@ -16,8 +16,9 @@
<https://www.gnu.org/licenses/>. */
#include <errno.h>
-#include <signal.h>
#include <internal-signals.h>
+#include <libc-lock.h>
+#include <signal.h>
/* 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 e2b84ba..75c6f34 100644
--- a/stdlib/abort.c
+++ b/stdlib/abort.c
@@ -15,13 +15,11 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-#include <libc-lock.h>
#include <signal.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
#include <internal-signals.h>
+#include <libc-lock.h>
+#include <pthreadP.h>
+#include <unistd.h>
/* Try to get a machine dependent instruction which will make the
program crash. This is used in case everything else fails. */
@@ -35,89 +33,54 @@
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;
-
-/* We should be prepared for multiple threads trying to run abort. */
-__libc_lock_define_initialized_recursive (static, lock);
-
+/* 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);
-/* Cause an abnormal program termination with core-dump. */
void
-abort (void)
+__abort_fork_reset_child (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);
+ __libc_lock_init (lock);
+}
- raise (SIGABRT);
+void
+__abort_lock_lock (internal_sigset_t *set)
+{
+ internal_signal_block_all (set);
+ __libc_lock_lock (lock);
+}
- __libc_lock_lock_recursive (lock);
- stage = save_stage + 1;
- }
+void
+__abort_lock_unlock (const internal_sigset_t *set)
+{
+ __libc_lock_unlock (lock);
+ internal_signal_restore_set (set);
+}
- /* 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);
- }
+/* Cause an abnormal program termination with core-dump. */
+_Noreturn void
+abort (void)
+{
+ raise (SIGABRT);
- /* Try again. */
- if (stage == 3)
- {
- ++stage;
- raise (SIGABRT);
- }
+ /* There is a SIGABRT handler 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. */
+ __abort_lock_lock (NULL);
- /* Now try to abort using the system specific command. */
- if (stage == 4)
- {
- ++stage;
- ABORT_INSTRUCTION;
- }
+ 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);
- /* If we can't signal ourselves and the abort instruction failed, exit. */
- if (stage == 5)
- {
- ++stage;
- _exit (127);
- }
+ /* This code should be unreachable, try the arch-specific code and the
+ syscall fallback. */
+ ABORT_INSTRUCTION;
- /* 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;
+ _exit (127);
}
libc_hidden_def (abort)
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 ef199dd..7d970e3 100644
--- a/sysdeps/nptl/_Fork.c
+++ b/sysdeps/nptl/_Fork.c
@@ -17,11 +17,17 @@
<https://www.gnu.org/licenses/>. */
#include <arch-fork.h>
+#include <libc-lock.h>
#include <pthreadP.h>
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 30e8a2d..b8ed954 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 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);