diff mbox

Refactor Linux raise implementation (BZ#15368)

Message ID 57658427.2090902@linaro.org
State Accepted
Commit 2ac88eecc57ff00e0b5ff803ebcc3465d2d640dd
Headers show

Commit Message

Adhemerval Zanella Netto June 18, 2016, 5:25 p.m. UTC
Thanks for reviews, comment below:

On 18/06/2016 11:09, Zack Weinberg wrote:
> On Fri, Jun 17, 2016 at 2:43 PM, Adhemerval Zanella

> <adhemerval.zanella@linaro.org> wrote:

>> This patch changes both the nptl and libc Linux raise implementation

>> to avoid the issues described in BZ#15368.

> 

> It would be nice to have comments in these files which explain why it

> is necessary to block all signals and why the cached values are not

> safe to use.  The old comment that you deleted ("raise is async-safe

> and could be called while fork/vfork temporarily invalidated the PID")

> would be a good start.

> 


Indeed, I did not remove them in this new version below. I also extended
the comment a bit explaining why to block the application signals.

>> The strategy used is

>> summarized in bug report first comment:

>>

>>  1. Block all signals (including internal NPTL ones);

> 

> The code appears to block all signals *except* the internal NPTL ones.

> If I understand Rich's description of the problem correctly, that is

> wrong.


Right, we need to block only user defined handler that might fork/vfork.
The call itself is OK, but I will change the comments describing why there
is no need to block GLIBC internal handlers.

> 

>> +static inline int

>> +__libc_signal_block_all (const sigset_t *set)

>> +{

> 

> Type-safety error here: the 'set' argument is written to and should

> not be 'const'.  It compiles because INTERNAL_SYSCALL() erases types,

> but the compiler would be entitled to assume that 'set' is unchanged

> after the call.


Ack.

> 

>> +static inline int

>> +__libc_signal_block_app (const sigset_t *set)

> 

> Same type-safety error here.


Ack.

> 

>> +  sigset_t set;

>> +  __libc_signal_block_app (&set);

>> +

>> +  pid_t pid = __getpid ();

> 

> If I'm reading the code correctly, __getpid does *not* bypass the cache.

> 

>> +  pid_t tid = INLINE_SYSCALL (gettid, 0);

> 

> INTERNAL_SYSCALL() here too?  gettid() can't fail, so there's no point

> generating the code to potentially set errno.

> 


Right, I will use INTERNAL_SYSCALL.

> zw

> 


I think this version addresses all the issues you raised:

--

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

--

Comments

Adhemerval Zanella Netto June 29, 2016, 12:57 p.m. UTC | #1
Ping.

On 18/06/2016 14:25, Adhemerval Zanella wrote:
> Thanks for reviews, comment below:

> 

> On 18/06/2016 11:09, Zack Weinberg wrote:

>> On Fri, Jun 17, 2016 at 2:43 PM, Adhemerval Zanella

>> <adhemerval.zanella@linaro.org> wrote:

>>> This patch changes both the nptl and libc Linux raise implementation

>>> to avoid the issues described in BZ#15368.

>>

>> It would be nice to have comments in these files which explain why it

>> is necessary to block all signals and why the cached values are not

>> safe to use.  The old comment that you deleted ("raise is async-safe

>> and could be called while fork/vfork temporarily invalidated the PID")

>> would be a good start.

>>

> 

> Indeed, I did not remove them in this new version below. I also extended

> the comment a bit explaining why to block the application signals.

> 

>>> The strategy used is

>>> summarized in bug report first comment:

>>>

>>>  1. Block all signals (including internal NPTL ones);

>>

>> The code appears to block all signals *except* the internal NPTL ones.

>> If I understand Rich's description of the problem correctly, that is

>> wrong.

> 

> Right, we need to block only user defined handler that might fork/vfork.

> The call itself is OK, but I will change the comments describing why there

> is no need to block GLIBC internal handlers.

> 

>>

>>> +static inline int

>>> +__libc_signal_block_all (const sigset_t *set)

>>> +{

>>

>> Type-safety error here: the 'set' argument is written to and should

>> not be 'const'.  It compiles because INTERNAL_SYSCALL() erases types,

>> but the compiler would be entitled to assume that 'set' is unchanged

>> after the call.

> 

> Ack.

> 

>>

>>> +static inline int

>>> +__libc_signal_block_app (const sigset_t *set)

>>

>> Same type-safety error here.

> 

> Ack.

> 

>>

>>> +  sigset_t set;

>>> +  __libc_signal_block_app (&set);

>>> +

>>> +  pid_t pid = __getpid ();

>>

>> If I'm reading the code correctly, __getpid does *not* bypass the cache.

>>

>>> +  pid_t tid = INLINE_SYSCALL (gettid, 0);

>>

>> INTERNAL_SYSCALL() here too?  gettid() can't fail, so there's no point

>> generating the code to potentially set errno.

>>

> 

> Right, I will use INTERNAL_SYSCALL.

> 

>> zw

>>

> 

> I think this version addresses all the issues you raised:

> 

> --

> 

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

> 

> --

> 

> diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/nptl-signals.h

> index 01f34c2..6525b6d 100644

> --- a/sysdeps/unix/sysv/linux/nptl-signals.h

> +++ b/sysdeps/unix/sysv/linux/nptl-signals.h

> @@ -39,5 +39,42 @@ __nptl_is_internal_signal (int sig)

>    return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID);

>  }

>  

> +static inline void

> +__nptl_clear_internal_signals (sigset_t *set)

> +{

> +  __sigdelset (set, SIGCANCEL);

> +  __sigdelset (set, SIGTIMER);

> +  __sigdelset (set, SIGSETXID);

> +}

> +

> +#define SIGALL_SET \

> +  ((__sigset_t) { .__val = {[0 ...  _SIGSET_NWORDS-1 ] =  -1 } })

> +

> +static inline int

> +__libc_signal_block_all (sigset_t *set)

> +{

> +  INTERNAL_SYSCALL_DECL (err);

> +  return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &SIGALL_SET,

> +			   set, _NSIG / 8);

> +}

> +

> +static inline int

> +__libc_signal_block_app (sigset_t *set)

> +{

> +  sigset_t allset = SIGALL_SET;

> +  __nptl_clear_internal_signals (&allset);

> +  INTERNAL_SYSCALL_DECL (err);

> +  return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &allset, set,

> +			   _NSIG / 8);

> +}

> +

> +static inline int

> +__libc_signal_restore_set (const sigset_t *set)

> +{

> +  INTERNAL_SYSCALL_DECL (err);

> +  return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_SETMASK, set, NULL,

> +			   _NSIG / 8);

> +}

> +

>  /* Used to communicate with signal handler.  */

>  extern struct xid_command *__xidcmd attribute_hidden;

> diff --git a/sysdeps/unix/sysv/linux/pt-raise.c b/sysdeps/unix/sysv/linux/pt-raise.c

> index 715bbe9..5f6dea1 100644

> --- a/sysdeps/unix/sysv/linux/pt-raise.c

> +++ b/sysdeps/unix/sysv/linux/pt-raise.c

> @@ -1,4 +1,5 @@

> -/* Copyright (C) 2002-2016 Free Software Foundation, Inc.

> +/* ISO C raise function for libpthread.

> +   Copyright (C) 2002-2016 Free Software Foundation, Inc.

>     This file is part of the GNU C Library.

>     Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.

>  

> @@ -16,22 +17,4 @@

>     License along with the GNU C Library; if not, see

>     <http://www.gnu.org/licenses/>.  */

>  

> -#include <errno.h>

> -#include <signal.h>

> -#include <sysdep.h>

> -#include <tls.h>

> -

> -

> -int

> -raise (int sig)

> -{

> -  /* raise is an async-safe function.  It could be called while the

> -     fork function temporarily invalidated the PID field.  Adjust for

> -     that.  */

> -  pid_t pid = THREAD_GETMEM (THREAD_SELF, pid);

> -  if (__glibc_unlikely (pid < 0))

> -    pid = -pid;

> -

> -  return INLINE_SYSCALL (tgkill, 3, pid, THREAD_GETMEM (THREAD_SELF, tid),

> -			 sig);

> -}

> +#include <sysdeps/unix/sysv/linux/raise.c>

> diff --git a/sysdeps/unix/sysv/linux/raise.c b/sysdeps/unix/sysv/linux/raise.c

> index 3795e6e..d386426 100644

> --- a/sysdeps/unix/sysv/linux/raise.c

> +++ b/sysdeps/unix/sysv/linux/raise.c

> @@ -16,42 +16,34 @@

>     License along with the GNU C Library; if not, see

>     <http://www.gnu.org/licenses/>.  */

>  

> -#include <errno.h>

> -#include <limits.h>

>  #include <signal.h>

>  #include <sysdep.h>

> -#include <nptl/pthreadP.h>

> -

> +#include <errno.h>

> +#include <sys/types.h>

> +#include <unistd.h>

> +#include <nptl-signals.h>

>  

>  int

>  raise (int sig)

>  {

> -  struct pthread *pd = THREAD_SELF;

> -  pid_t pid = THREAD_GETMEM (pd, pid);

> -  pid_t selftid = THREAD_GETMEM (pd, tid);

> -  if (selftid == 0)

> -    {

> -      /* This system call is not supposed to fail.  */

> -#ifdef INTERNAL_SYSCALL

> -      INTERNAL_SYSCALL_DECL (err);

> -      selftid = INTERNAL_SYSCALL (gettid, err, 0);

> -#else

> -      selftid = INLINE_SYSCALL (gettid, 0);

> -#endif

> -      THREAD_SETMEM (pd, tid, selftid);

> -

> -      /* We do not set the PID field in the TID here since we might be

> -	 called from a signal handler while the thread executes fork.  */

> -      pid = selftid;

> -    }

> -  else

> -    /* raise is an async-safe function.  It could be called while the

> -       fork/vfork function temporarily invalidated the PID field.  Adjust for

> -       that.  */

> -    if (__glibc_unlikely (pid <= 0))

> -      pid = (pid & INT_MAX) == 0 ? selftid : -pid;

> -

> -  return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);

> +  /* raise is an async-safe function so it could be called while the

> +     fork/vfork function temporarily invalidated the PID field.  To avoid

> +     relying in the cached value we block all user-defined signal handler

> +     (which might call fork/vfork) and issues the getpid and gettid

> +     directly.  */

> +

> +  sigset_t set;

> +  __libc_signal_block_app (&set);

> +

> +  INTERNAL_SYSCALL_DECL (err);

> +  pid_t pid = INTERNAL_SYSCALL (getpid, err, 0);

> +  pid_t tid = INTERNAL_SYSCALL (gettid, err, 0);

> +

> +  int ret = INLINE_SYSCALL (tgkill, 3, pid, tid, sig);

> +

> +  __libc_signal_restore_set (&set);

> +

> +  return ret;

>  }

>  libc_hidden_def (raise)

>  weak_alias (raise, gsignal)

>
Adhemerval Zanella Netto July 7, 2016, 3:16 p.m. UTC | #2
Ping x2 (I would like to push it before hard freeze).

On 29/06/2016 09:57, Adhemerval Zanella wrote:
> Ping.

> 

> On 18/06/2016 14:25, Adhemerval Zanella wrote:

>> Thanks for reviews, comment below:

>>

>> On 18/06/2016 11:09, Zack Weinberg wrote:

>>> On Fri, Jun 17, 2016 at 2:43 PM, Adhemerval Zanella

>>> <adhemerval.zanella@linaro.org> wrote:

>>>> This patch changes both the nptl and libc Linux raise implementation

>>>> to avoid the issues described in BZ#15368.

>>>

>>> It would be nice to have comments in these files which explain why it

>>> is necessary to block all signals and why the cached values are not

>>> safe to use.  The old comment that you deleted ("raise is async-safe

>>> and could be called while fork/vfork temporarily invalidated the PID")

>>> would be a good start.

>>>

>>

>> Indeed, I did not remove them in this new version below. I also extended

>> the comment a bit explaining why to block the application signals.

>>

>>>> The strategy used is

>>>> summarized in bug report first comment:

>>>>

>>>>  1. Block all signals (including internal NPTL ones);

>>>

>>> The code appears to block all signals *except* the internal NPTL ones.

>>> If I understand Rich's description of the problem correctly, that is

>>> wrong.

>>

>> Right, we need to block only user defined handler that might fork/vfork.

>> The call itself is OK, but I will change the comments describing why there

>> is no need to block GLIBC internal handlers.

>>

>>>

>>>> +static inline int

>>>> +__libc_signal_block_all (const sigset_t *set)

>>>> +{

>>>

>>> Type-safety error here: the 'set' argument is written to and should

>>> not be 'const'.  It compiles because INTERNAL_SYSCALL() erases types,

>>> but the compiler would be entitled to assume that 'set' is unchanged

>>> after the call.

>>

>> Ack.

>>

>>>

>>>> +static inline int

>>>> +__libc_signal_block_app (const sigset_t *set)

>>>

>>> Same type-safety error here.

>>

>> Ack.

>>

>>>

>>>> +  sigset_t set;

>>>> +  __libc_signal_block_app (&set);

>>>> +

>>>> +  pid_t pid = __getpid ();

>>>

>>> If I'm reading the code correctly, __getpid does *not* bypass the cache.

>>>

>>>> +  pid_t tid = INLINE_SYSCALL (gettid, 0);

>>>

>>> INTERNAL_SYSCALL() here too?  gettid() can't fail, so there's no point

>>> generating the code to potentially set errno.

>>>

>>

>> Right, I will use INTERNAL_SYSCALL.

>>

>>> zw

>>>

>>

>> I think this version addresses all the issues you raised:

>>

>> --

>>

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

>>

>> --

>>

>> diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/nptl-signals.h

>> index 01f34c2..6525b6d 100644

>> --- a/sysdeps/unix/sysv/linux/nptl-signals.h

>> +++ b/sysdeps/unix/sysv/linux/nptl-signals.h

>> @@ -39,5 +39,42 @@ __nptl_is_internal_signal (int sig)

>>    return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID);

>>  }

>>  

>> +static inline void

>> +__nptl_clear_internal_signals (sigset_t *set)

>> +{

>> +  __sigdelset (set, SIGCANCEL);

>> +  __sigdelset (set, SIGTIMER);

>> +  __sigdelset (set, SIGSETXID);

>> +}

>> +

>> +#define SIGALL_SET \

>> +  ((__sigset_t) { .__val = {[0 ...  _SIGSET_NWORDS-1 ] =  -1 } })

>> +

>> +static inline int

>> +__libc_signal_block_all (sigset_t *set)

>> +{

>> +  INTERNAL_SYSCALL_DECL (err);

>> +  return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &SIGALL_SET,

>> +			   set, _NSIG / 8);

>> +}

>> +

>> +static inline int

>> +__libc_signal_block_app (sigset_t *set)

>> +{

>> +  sigset_t allset = SIGALL_SET;

>> +  __nptl_clear_internal_signals (&allset);

>> +  INTERNAL_SYSCALL_DECL (err);

>> +  return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &allset, set,

>> +			   _NSIG / 8);

>> +}

>> +

>> +static inline int

>> +__libc_signal_restore_set (const sigset_t *set)

>> +{

>> +  INTERNAL_SYSCALL_DECL (err);

>> +  return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_SETMASK, set, NULL,

>> +			   _NSIG / 8);

>> +}

>> +

>>  /* Used to communicate with signal handler.  */

>>  extern struct xid_command *__xidcmd attribute_hidden;

>> diff --git a/sysdeps/unix/sysv/linux/pt-raise.c b/sysdeps/unix/sysv/linux/pt-raise.c

>> index 715bbe9..5f6dea1 100644

>> --- a/sysdeps/unix/sysv/linux/pt-raise.c

>> +++ b/sysdeps/unix/sysv/linux/pt-raise.c

>> @@ -1,4 +1,5 @@

>> -/* Copyright (C) 2002-2016 Free Software Foundation, Inc.

>> +/* ISO C raise function for libpthread.

>> +   Copyright (C) 2002-2016 Free Software Foundation, Inc.

>>     This file is part of the GNU C Library.

>>     Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.

>>  

>> @@ -16,22 +17,4 @@

>>     License along with the GNU C Library; if not, see

>>     <http://www.gnu.org/licenses/>.  */

>>  

>> -#include <errno.h>

>> -#include <signal.h>

>> -#include <sysdep.h>

>> -#include <tls.h>

>> -

>> -

>> -int

>> -raise (int sig)

>> -{

>> -  /* raise is an async-safe function.  It could be called while the

>> -     fork function temporarily invalidated the PID field.  Adjust for

>> -     that.  */

>> -  pid_t pid = THREAD_GETMEM (THREAD_SELF, pid);

>> -  if (__glibc_unlikely (pid < 0))

>> -    pid = -pid;

>> -

>> -  return INLINE_SYSCALL (tgkill, 3, pid, THREAD_GETMEM (THREAD_SELF, tid),

>> -			 sig);

>> -}

>> +#include <sysdeps/unix/sysv/linux/raise.c>

>> diff --git a/sysdeps/unix/sysv/linux/raise.c b/sysdeps/unix/sysv/linux/raise.c

>> index 3795e6e..d386426 100644

>> --- a/sysdeps/unix/sysv/linux/raise.c

>> +++ b/sysdeps/unix/sysv/linux/raise.c

>> @@ -16,42 +16,34 @@

>>     License along with the GNU C Library; if not, see

>>     <http://www.gnu.org/licenses/>.  */

>>  

>> -#include <errno.h>

>> -#include <limits.h>

>>  #include <signal.h>

>>  #include <sysdep.h>

>> -#include <nptl/pthreadP.h>

>> -

>> +#include <errno.h>

>> +#include <sys/types.h>

>> +#include <unistd.h>

>> +#include <nptl-signals.h>

>>  

>>  int

>>  raise (int sig)

>>  {

>> -  struct pthread *pd = THREAD_SELF;

>> -  pid_t pid = THREAD_GETMEM (pd, pid);

>> -  pid_t selftid = THREAD_GETMEM (pd, tid);

>> -  if (selftid == 0)

>> -    {

>> -      /* This system call is not supposed to fail.  */

>> -#ifdef INTERNAL_SYSCALL

>> -      INTERNAL_SYSCALL_DECL (err);

>> -      selftid = INTERNAL_SYSCALL (gettid, err, 0);

>> -#else

>> -      selftid = INLINE_SYSCALL (gettid, 0);

>> -#endif

>> -      THREAD_SETMEM (pd, tid, selftid);

>> -

>> -      /* We do not set the PID field in the TID here since we might be

>> -	 called from a signal handler while the thread executes fork.  */

>> -      pid = selftid;

>> -    }

>> -  else

>> -    /* raise is an async-safe function.  It could be called while the

>> -       fork/vfork function temporarily invalidated the PID field.  Adjust for

>> -       that.  */

>> -    if (__glibc_unlikely (pid <= 0))

>> -      pid = (pid & INT_MAX) == 0 ? selftid : -pid;

>> -

>> -  return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);

>> +  /* raise is an async-safe function so it could be called while the

>> +     fork/vfork function temporarily invalidated the PID field.  To avoid

>> +     relying in the cached value we block all user-defined signal handler

>> +     (which might call fork/vfork) and issues the getpid and gettid

>> +     directly.  */

>> +

>> +  sigset_t set;

>> +  __libc_signal_block_app (&set);

>> +

>> +  INTERNAL_SYSCALL_DECL (err);

>> +  pid_t pid = INTERNAL_SYSCALL (getpid, err, 0);

>> +  pid_t tid = INTERNAL_SYSCALL (gettid, err, 0);

>> +

>> +  int ret = INLINE_SYSCALL (tgkill, 3, pid, tid, sig);

>> +

>> +  __libc_signal_restore_set (&set);

>> +

>> +  return ret;

>>  }

>>  libc_hidden_def (raise)

>>  weak_alias (raise, gsignal)

>>
Adhemerval Zanella Netto July 7, 2016, 4:12 p.m. UTC | #3
On 07/07/2016 12:30, Andreas Schwab wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:

> 

>> +  /* raise is an async-safe function so it could be called while the

>> +     fork/vfork function temporarily invalidated the PID field.  To avoid

>> +     relying in the cached value we block all user-defined signal handler

>> +     (which might call fork/vfork) and issues the getpid and gettid

>> +     directly.  */

> 

> s/issues/issue/; s/^/ syscalls/


Right, I will fix it.

> 

>> +  sigset_t set;

>> +  __libc_signal_block_app (&set);

>> +

>> +  INTERNAL_SYSCALL_DECL (err);

>> +  pid_t pid = INTERNAL_SYSCALL (getpid, err, 0);

>> +  pid_t tid = INTERNAL_SYSCALL (gettid, err, 0);

>> +

>> +  int ret = INLINE_SYSCALL (tgkill, 3, pid, tid, sig);

>> +

>> +  __libc_signal_restore_set (&set);

> 

> What if block/unblock fail?


My understanding checking on kernel source is 'rt_sigprocmask' may fail if:

  1. sigsetsize != sizeof (sigset_t) (EINVAL)
  2. a failure in copy_from_user/copy_to_user (EFAULT)
  3. an invalid 'how' operation (EINVAL)

The first case is already handle in glibc syscall call by using the arch 
defined _NSIG.  Second is handled by using a stack allocated mask in 'raise'
implementation.  The last one should be handled by the
__libc_signal_{un}block_{app,all} macros.

I think there is no need in this specific usage to handle failures.
Adhemerval Zanella Netto July 11, 2016, 8:05 a.m. UTC | #4
On 11/07/2016 04:07, Andreas Schwab wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:

> 

>> I think there is no need in this specific usage to handle failures.

> 

> Could you please add a comment to that effect?

> 

> Andreas.

> 


I will do it.
diff mbox

Patch

diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/nptl-signals.h
index 01f34c2..6525b6d 100644
--- a/sysdeps/unix/sysv/linux/nptl-signals.h
+++ b/sysdeps/unix/sysv/linux/nptl-signals.h
@@ -39,5 +39,42 @@  __nptl_is_internal_signal (int sig)
   return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID);
 }
 
+static inline void
+__nptl_clear_internal_signals (sigset_t *set)
+{
+  __sigdelset (set, SIGCANCEL);
+  __sigdelset (set, SIGTIMER);
+  __sigdelset (set, SIGSETXID);
+}
+
+#define SIGALL_SET \
+  ((__sigset_t) { .__val = {[0 ...  _SIGSET_NWORDS-1 ] =  -1 } })
+
+static inline int
+__libc_signal_block_all (sigset_t *set)
+{
+  INTERNAL_SYSCALL_DECL (err);
+  return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &SIGALL_SET,
+			   set, _NSIG / 8);
+}
+
+static inline int
+__libc_signal_block_app (sigset_t *set)
+{
+  sigset_t allset = SIGALL_SET;
+  __nptl_clear_internal_signals (&allset);
+  INTERNAL_SYSCALL_DECL (err);
+  return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_BLOCK, &allset, set,
+			   _NSIG / 8);
+}
+
+static inline int
+__libc_signal_restore_set (const sigset_t *set)
+{
+  INTERNAL_SYSCALL_DECL (err);
+  return INTERNAL_SYSCALL (rt_sigprocmask, err, 4, SIG_SETMASK, set, NULL,
+			   _NSIG / 8);
+}
+
 /* Used to communicate with signal handler.  */
 extern struct xid_command *__xidcmd attribute_hidden;
diff --git a/sysdeps/unix/sysv/linux/pt-raise.c b/sysdeps/unix/sysv/linux/pt-raise.c
index 715bbe9..5f6dea1 100644
--- a/sysdeps/unix/sysv/linux/pt-raise.c
+++ b/sysdeps/unix/sysv/linux/pt-raise.c
@@ -1,4 +1,5 @@ 
-/* Copyright (C) 2002-2016 Free Software Foundation, Inc.
+/* ISO C raise function for libpthread.
+   Copyright (C) 2002-2016 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
 
@@ -16,22 +17,4 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <signal.h>
-#include <sysdep.h>
-#include <tls.h>
-
-
-int
-raise (int sig)
-{
-  /* raise is an async-safe function.  It could be called while the
-     fork function temporarily invalidated the PID field.  Adjust for
-     that.  */
-  pid_t pid = THREAD_GETMEM (THREAD_SELF, pid);
-  if (__glibc_unlikely (pid < 0))
-    pid = -pid;
-
-  return INLINE_SYSCALL (tgkill, 3, pid, THREAD_GETMEM (THREAD_SELF, tid),
-			 sig);
-}
+#include <sysdeps/unix/sysv/linux/raise.c>
diff --git a/sysdeps/unix/sysv/linux/raise.c b/sysdeps/unix/sysv/linux/raise.c
index 3795e6e..d386426 100644
--- a/sysdeps/unix/sysv/linux/raise.c
+++ b/sysdeps/unix/sysv/linux/raise.c
@@ -16,42 +16,34 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <limits.h>
 #include <signal.h>
 #include <sysdep.h>
-#include <nptl/pthreadP.h>
-
+#include <errno.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <nptl-signals.h>
 
 int
 raise (int sig)
 {
-  struct pthread *pd = THREAD_SELF;
-  pid_t pid = THREAD_GETMEM (pd, pid);
-  pid_t selftid = THREAD_GETMEM (pd, tid);
-  if (selftid == 0)
-    {
-      /* This system call is not supposed to fail.  */
-#ifdef INTERNAL_SYSCALL
-      INTERNAL_SYSCALL_DECL (err);
-      selftid = INTERNAL_SYSCALL (gettid, err, 0);
-#else
-      selftid = INLINE_SYSCALL (gettid, 0);
-#endif
-      THREAD_SETMEM (pd, tid, selftid);
-
-      /* We do not set the PID field in the TID here since we might be
-	 called from a signal handler while the thread executes fork.  */
-      pid = selftid;
-    }
-  else
-    /* raise is an async-safe function.  It could be called while the
-       fork/vfork function temporarily invalidated the PID field.  Adjust for
-       that.  */
-    if (__glibc_unlikely (pid <= 0))
-      pid = (pid & INT_MAX) == 0 ? selftid : -pid;
-
-  return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
+  /* raise is an async-safe function so it could be called while the
+     fork/vfork function temporarily invalidated the PID field.  To avoid
+     relying in the cached value we block all user-defined signal handler
+     (which might call fork/vfork) and issues the getpid and gettid
+     directly.  */
+
+  sigset_t set;
+  __libc_signal_block_app (&set);
+
+  INTERNAL_SYSCALL_DECL (err);
+  pid_t pid = INTERNAL_SYSCALL (getpid, err, 0);
+  pid_t tid = INTERNAL_SYSCALL (gettid, err, 0);
+
+  int ret = INLINE_SYSCALL (tgkill, 3, pid, tid, sig);
+
+  __libc_signal_restore_set (&set);
+
+  return ret;
 }
 libc_hidden_def (raise)
 weak_alias (raise, gsignal)