Bug 15368 - raise() is not async-signal-safe
Summary: raise() is not async-signal-safe
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: nptl (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: 2.24
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-15 02:50 UTC by Rich Felker
Modified: 2016-07-14 08:16 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Rich Felker 2013-04-15 02:50:54 UTC
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.
Comment 1 Carlos O'Donell 2016-04-15 04:50:55 UTC
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).
Comment 2 cvs-commit@gcc.gnu.org 2016-07-13 16:10:35 UTC
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(-)
Comment 3 Adhemerval Zanella 2016-07-14 08:16:03 UTC
Fixed by 2ac88eecc57ff00e0b5ff803ebcc3465d2d640dd.