stdlib: Make abort AS-safe (BZ 26275)

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.

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

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)

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}.

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)
 {

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

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)

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;
+/* 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);
 
-/* We should be prepared for multiple threads trying to run abort.  */
-__libc_lock_define_initialized_recursive (static, lock);
+void
+__abort_fork_reset_child (void)
+{
+  __libc_lock_init (lock);
+}
 
+void
+__abort_lock_lock (internal_sigset_t *set)
+{
+  internal_signal_block_all (set);
+  __libc_lock_lock (lock);
+}
+
+void
+__abort_lock_unlock (const internal_sigset_t *set)
+{
+  __libc_lock_unlock (lock);
+  internal_signal_restore_set (set);
+}
 
 /* Cause an abnormal program termination with core-dump.  */
-void
+_Noreturn void
 abort (void)
 {
-  struct sigaction act;
+  raise (SIGABRT);
 
-  /* First acquire the lock.  */
-  __libc_lock_lock_recursive (lock);
+  /* 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 it's for sure we are alone.  But recursive calls are possible.  */
+  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);
 
-  /* Unblock SIGABRT.  */
-  if (stage == 0)
-    {
-      ++stage;
-      internal_sigset_t sigs;
-      internal_sigemptyset (&sigs);
-      internal_sigaddset (&sigs, SIGABRT);
-      internal_sigprocmask (SIG_UNBLOCK, &sigs, NULL);
-    }
+  /* This code should be unreachable, try the arch-specific code and the
+     syscall fallback.  */
+  ABORT_INSTRUCTION;
 
-  /* 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);
-
-      raise (SIGABRT);
-
-      __libc_lock_lock_recursive (lock);
-      stage = save_stage + 1;
-    }
-
-  /* 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);
-    }
-
-  /* Try again.  */
-  if (stage == 3)
-    {
-      ++stage;
-      raise (SIGABRT);
-    }
-
-  /* Now try to abort using the system specific command.  */
-  if (stage == 4)
-    {
-      ++stage;
-      ABORT_INSTRUCTION;
-    }
-
-  /* If we can't signal ourselves and the abort instruction failed, exit.  */
-  if (stage == 5)
-    {
-      ++stage;
-      _exit (127);
-    }
-
-  /* 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)

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  */

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

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)

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)

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

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.  */

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;

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