Refactor Linux raise implementation (BZ#15368)
Commit Message
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
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)
>
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 <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/
> + 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?
Andreas.
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 <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.
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.
@@ -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;
@@ -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>
@@ -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)