Re: [PATCH] Refactor Linux raise implementation (BZ#15368)

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

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.