mbox series

[v2,0/2] Make abort AS-safe

Message ID 20230803173436.4146900-1-adhemerval.zanella@linaro.org
Headers show
Series Make abort AS-safe | expand

Message

Adhemerval Zanella Netto Aug. 3, 2023, 5:34 p.m. UTC 
Besides POSIX stating abort should be AS-safe, Rust also had an
open PR about it [1] (it was closed with a different fix).

The main issue is 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 raising SIGABRT might create
a new process with a non-expected signal mask.  The requirement of
SIGABRT handler being called even if the signals is blocked was
changed by a POSIX defect [3]

To fix the AS-safe, the raise is issued 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 used on
both _Fork and posix_spawn, to avoid the spawn process to see the
abort handler as SIG_DFL.

The clone is also subjected to this issue, but since glibc does not
do any internal metadata setup (as for fork-like function), this patch
does not handle it for the symbol.

I have not added a regression tests because, from previous Carlos's
patch [2], hitting the code path to trigger the potential issue
(fork just after abort has acquired the lock and reset SIGABRT handler)
is not deterministic and it would generate a lot of development
overhead.

[1]
https://github.com/rust-lang/rust/issues/73894#issuecomment-673478761
[2]
https://sourceware.org/pipermail/libc-alpha/2020-September/117934.html
[3] https://austingroupbugs.net/view.php?id=906#c5851

Changes from v1:
- Change __abort_lock_lock/__abort_lock_unlock to use a 
  internal_sigset_t.
- Improve comments.
- Use gettid syscall on __pthread_raise_internal to work after vfork.

Adhemerval Zanella (2):
  setjmp: Use BSD sematic as default for setjmp
  stdlib: Make abort AS-safe (BZ 26275)

 include/bits/unistd_ext.h                  |   3 +
 include/stdlib.h                           |   6 +
 manual/setjmp.texi                         |  14 +--
 manual/startup.texi                        |   3 -
 nptl/pthread_create.c                      |   3 +-
 nptl/pthread_kill.c                        |  11 ++
 posix/fork.c                               |   2 +
 setjmp/setjmp.h                            |   5 -
 signal/sigaction.c                         |  15 ++-
 stdlib/abort.c                             | 130 ++++++++-------------
 sysdeps/generic/internal-signals.h         |  27 ++++-
 sysdeps/generic/internal-sigset.h          |  26 +++++
 sysdeps/htl/pthreadP.h                     |   2 +
 sysdeps/nptl/_Fork.c                       |   9 ++
 sysdeps/nptl/libc_start_call_main.h        |   3 +-
 sysdeps/nptl/pthreadP.h                    |   1 +
 sysdeps/unix/sysv/linux/internal-signals.h |   9 ++
 sysdeps/unix/sysv/linux/internal-sigset.h  |   2 +-
 sysdeps/unix/sysv/linux/spawni.c           |   6 +-
 19 files changed, 167 insertions(+), 110 deletions(-)
 create mode 100644 sysdeps/generic/internal-sigset.h