NPTL's raise() is implemented as a call to pthread_kill(pthread_self(), sig); this implementation is invalid because pthread_kill is not async-signal-safe. In particular, if a signal handler forks during raise, the the thread in the parent could receive the signal twice, once from itself and once from the child. Or, if raise() is called from a signal handler while fork() has temporarily modified the tid/pid in TLS, raise() may fail to do anything. Other incorrect behavior may also be possible too; I have not analyzed the issue in detail. The (only) correct implementation of raise() for multi-threaded programs is: 1. Block all signals. 2. Get pid and tid from the kernel via syscalls. 3. Make the tgkill syscall. 4. Restore the old signal mask. Blocking signals is required to make obtaining the tid/pid atomic with sending the signal. Anything else fails to meet the async-signal safety requirements.
This issue still applies in general. The point about double signal delivery is valid (and worrisome). The issues about raise() doing nothing are not correct since raise() coordinates with fork() and vfork() over the use of -PID. However, clone() sets PID/TID to -1/-1 and that will certainly break a raise() in a signal handler delivered to the child of the clone so that's a bug (one which will be fixed soon probably by removing setting pid/tid to -1/-1 in clone).
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "GNU C Library master sources". The branch, master has been updated via 2ac88eecc57ff00e0b5ff803ebcc3465d2d640dd (commit) from e15eaa8f335ebfd565ab7752c64f3415d427d9b2 (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=2ac88eecc57ff00e0b5ff803ebcc3465d2d640dd commit 2ac88eecc57ff00e0b5ff803ebcc3465d2d640dd Author: Adhemerval Zanella <adhemerval.zanella@linaro.org> Date: Fri Apr 22 09:25:20 2016 -0300 Refactor Linux raise implementation (BZ#15368) This patch changes both the nptl and libc Linux raise implementation to avoid the issues described in BZ#15368. The strategy used is summarized in bug report first comment: 1. Block all signals (including internal NPTL ones); 2. Get pid and tid directly from syscall (not relying on cached values); 3. Call tgkill; 4. Restore old signal mask. Tested on x86_64 and i686. [BZ #15368] * sysdeps/unix/sysv/linux/nptl-signals.h (__nptl_clear_internal_signals): New function. (__libc_signal_block_all): Likewise. (__libc_signal_block_app): Likewise. (__libc_signal_restore_set): Likewise. * sysdeps/unix/sysv/linux/pt-raise.c (raise): Use Linux raise.c implementation. * sysdeps/unix/sysv/linux/raise.c (raise): Reimplement to not use the cached pid/tid value in pthread structure. ----------------------------------------------------------------------- Summary of changes: ChangeLog | 13 +++++++ sysdeps/unix/sysv/linux/nptl-signals.h | 41 +++++++++++++++++++++ sysdeps/unix/sysv/linux/pt-raise.c | 23 ++---------- sysdeps/unix/sysv/linux/raise.c | 63 +++++++++++++++++--------------- 4 files changed, 90 insertions(+), 50 deletions(-)
Fixed by 2ac88eecc57ff00e0b5ff803ebcc3465d2d640dd.
This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "GNU C Library master sources". The branch, zack/wip-check-localplt-2 has been created at c6c5c2887d8b6883ee69a1e183119f2be02e660f (commit) - Log ----------------------------------------------------------------- https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=c6c5c2887d8b6883ee69a1e183119f2be02e660f commit c6c5c2887d8b6883ee69a1e183119f2be02e660f Author: Zack Weinberg <zackw@panix.com> Date: Sun Mar 18 17:01:06 2018 -0400 WIP finer-grained, more aggressive local PLT call check https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=9ea49e16c79bd2acd0d0648ca0163f26dd1c3dae commit 9ea49e16c79bd2acd0d0648ca0163f26dd1c3dae Author: Zack Weinberg <zackw@panix.com> Date: Fri Mar 23 09:16:59 2018 -0400 [Bug 15368] Move pthread_kill to libc and use it to implement raise. The fix for bug #15368 was unnecessarily Linux-specific. To recap, POSIX specifies raise to be async-signal-safe, but also specifies it to be equivalent to pthread_kill(pthread_self(), sig), which is not an async-signal-safe sequence of operations; a signal handler could run in between pthread_self and pthread_kill, and do something (such as calling fork, which is also async-signal-safe) that would invalidate the thread descriptor. This is even true in the hypothetical case of a port that doesn't implement multithreading: kill(getpid(), sig) will fire the signal twice if a signal handler runs in between, calls fork, and then returns on both sides of the fork. I don't see anything in the standards to forbid that. The Linux-specific fix was to override the definitions of raise in both libpthread and libc to the same unitary function that blocks signals, retrieves TID and PID directly from the kernel, calls tgkill, and only then unblocks signals. This patch generalizes that to any port: pthread_kill is moved from libpthread to libc, with a forwarding stub left behind. The definition of raise in libpthread is also replaced with a forwarding stub. The Linux-specific definition of raise is deleted; those ports will now use sysdeps/pthread/raise.c, which blocks signals first, then calls pthread_self and pthread_kill, and then unblocks signals. Similarly, sysdeps/posix/raise.c (which would be used on a port that didn't implement multithreading) blocks signals, calls getpid and kill, and then unblocks signals. Thus, ports need only implement the primitives correctly and do not need to worry about making raise async-signal-safe. The only wrinkle was that up till now, we did not bother initializing the ->tid field of the initial thread's descriptor unless libpthread was loaded; now that raise calls pthread_kill even in a single- threaded environment, that won't fly. This is abstractly easy to fix; the tricky part was figuring out _where_ to put the calls (two of them, as it happens) to __pthread_initialize_pids, and I'd appreciate careful eyes on those changes. You might be wondering why it's safe to rely on the TID in the thread descriptor, rather than calling gettid directly. Since all signals are blocked from before calling pthread_self until after pthread_kill uses the TID to call tgkill, the question is whether some _other_ thread could do something that would invalidate the calling thread's descriptor, and I believe there is no such thing. While I was at it I fixed another bug: raise was returning an error code on failure (like pthread_kill does) instead of setting errno as specified. This is user-visible but I don't think it's worth recording as a fixed bug, nobody bothers checking whether raise failed anyway. * nptl/pt-raise.c * sysdeps/unix/sysv/linux/pt-raise.c * sysdeps/unix/sysv/linux/raise.c: Remove file. * sysdeps/unix/sysv/linux/pthread_kill.c: Use __is_internal_signal to check for forbidden signals. Use INTERNAL_SYSCALL_CALL to call getpid. Provide __libc_pthread_kill, with __pthread_kill as strong alias and pthread_kill as weak alias. * sysdeps/posix/raise.c: Block signals around the calls to __getpid and __kill. Provide __libc_raise, with raise as strong alias, libc_hidden_def for raise, and gsignal as weak alias. * sysdeps/pthread/raise.c: New file. Implement by blocking signals, calling pthread_self and pthread_kill, and then unblocking signals again. Provide same symbols as above. * sysdeps/generic/internal-signals.h: Define all of the same functions that sysdeps/unix/sysv/linux/internal-signals.h does, with sensible default definitions. * sysdeps/unix/sysv/linux/internal-signals.h: Clarify comments. * nptl/pthread_kill.c: Define __libc_pthread_kill, with __pthread_kill as strong alias and pthread_kill as weak alias. * nptl/pthread_self.c: Define __pthread_self, with pthread_self as weak alias. * signal/raise.c: Define __libc_raise, with raise as strong alias, libc_hidden_def for raise, and gsignal as weak alias. * nptl/Makefile: Move pthread_kill from libpthread-routines to routines. Remove pt-raise from libpthread-routines. * nptl/Versions (libc/GLIBC_2.28): Add pthread_kill. (libc/GLIBC_PRIVATE): Add __libc_pthread_kill and __libc_raise. * sysdeps/generic/pt-compat-stubs.S: Add stubs for raise and pthread_kill. * nptl/nptl-init.c (__pthread_initialize_minimal_internal): Don't call __pthread_initialize_pids here. * csu/libc-tls.c (__libc_setup_tls): Call __pthread_initialize_pids after all other setup. * elf/rtld.c (init_tls): Likewise. * include/pthreadP.h: New forwarder. * include/pthread.h: Add multiple inclusion guard. Declare __pthread_self. * include/signal.h: Declare __pthread_kill. * sysdeps/**/libc.abilist (GLIBC_2.28): Add pthread_kill. https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=3d8eb8099425ae4f474e97082e04784c2984ec48 commit 3d8eb8099425ae4f474e97082e04784c2984ec48 Author: Zack Weinberg <zackw@panix.com> Date: Fri Mar 23 16:17:26 2018 -0400 WIP no duplicate function definitions in pthread https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=65a7c4c9272f1b14a2b87733bf4dbb564c3dd1d2 commit 65a7c4c9272f1b14a2b87733bf4dbb564c3dd1d2 Author: Zack Weinberg <zackw@panix.com> Date: Sun Mar 18 17:12:42 2018 -0400 RFC: Introduce pt-compat-stubs and use it to replace pt-vfork. I am looking into the possibility of eliminating all of the duplicate function definitions from libpthread, replacing them with properly-tagged weak compatibility symbols that just call the definition in libc. Because one of the duplicated functions is vfork, the calls to libc absolutely must reuse the stack frame (a "sibcall" in GCC internals jargon), and on several important targets, GCC does not implement sibcalls, or only implements them for intra-module calls. But we only need to implement a single special case, sibcalling a function with exactly the same signature, from immediately after the caller's own entry point; so doing it by hand in assembly language is not a crazy notion. I believe I have managed to turn the trick for all currently-supported targets. This patch just converts the existing vfork stub, so that review can focus on the new sysdep.h SIBCALL macros. * sysdeps/generic/pt-compat-stubs.S: New file. * nptl/Makefile (libpthread-routines): Remove pt-vfork, add pt-compat-stubs. (libpthread-shared-only-routines): Add pt-compat-stubs. * posix/vfork.c: Define __libc_vfork as well as __vfork and vfork. * sysdeps/generic/sysdep.h (SIBCALL): New macro to perform sibling calls; the generic definition errors out if used. * sysdeps/aarch64/sysdep.h, sysdeps/arm/sysdep.h * sysdeps/hppa/sysdep.h, sysdeps/ia64/sysdep.h * sysdeps/m68k/sysdep.h, sysdeps/microblaze/sysdep.h * sysdeps/nios2/sysdep.h, sysdeps/powerpc/powerpc32/sysdep.h * sysdeps/powerpc/powerpc64/sysdep.h, sysdeps/s390/s390-32/sysdep.h * sysdeps/s390/s390-64/sysdep.h, sysdeps/tile/sysdep.h * sysdeps/unix/alpha/sysdep.h, sysdeps/unix/mips/mips32/sysdep.h * sysdeps/unix/mips/mips64/n32/sysdep.h * sysdeps/unix/mips/mips64/n64/sysdep.h * sysdeps/unix/sysv/linux/riscv/sysdep.h * sysdeps/x86/sysdep.h Provide appropriate architecture-specific definitions of SIBCALL and, if necessary, SIBCALL_ENTRY. * nptl/pt-vfork.c * sysdeps/unix/sysv/linux/aarch64/pt-vfork.c * sysdeps/unix/sysv/linux/m68k/pt-vfork.c * sysdeps/unix/sysv/linux/tile/pt-vfork.c * sysdeps/unix/sysv/linux/alpha/pt-vfork.S * sysdeps/unix/sysv/linux/hppa/pt-vfork.S * sysdeps/unix/sysv/linux/ia64/pt-vfork.S * sysdeps/unix/sysv/linux/microblaze/pt-vfork.S * sysdeps/unix/sysv/linux/mips/pt-vfork.S * sysdeps/unix/sysv/linux/riscv/pt-vfork.S * sysdeps/unix/sysv/linux/s390/pt-vfork.S * sysdeps/unix/sysv/linux/sh/pt-vfork.S * sysdeps/unix/sysv/linux/sparc/pt-vfork.S Remove file. -----------------------------------------------------------------------