Re: [PATCH 6/7] Block all signals on timer_create thread (BZ#10815)

[Florian Weimer <fweimer at redhat dot com>]

* Adhemerval Zanella:

> The behavior of the signal mask on threads created by timer_create
> for SIGEV_THREAD timers are implementation defined and glibc explicit
                              implementation-defined           explictlly

> unblocks all signals before calling the user-defined function.
>
> This behavior, although not incorrect standard-wise, opens a race if a
> program using a blocked rt-signal plus sigwaitinfo (and without an
> installed signal handler for the rt-signal) receives the signal while
> executing the timer threads created by SIGEV_THREAD.

while executing the user-defined function for SIGEV_THREAD?

> A better alternative discussed in bug report is to rather block all
> signals (besides the internal ones not available to application
> usage).
>
> This patch fixes by only unblock the SIGSETXID (used on set*uid).
             fixes this issiue by only unblicking SIGSETXID

> The SIGTIMER is the same as SIGCANCEL and it will be handled by

No “The”?

> sigwaitinfo on the helper thread (thus it can be masked as well).
>
> Checked on x86_64-linux-gnu and i686-linux-gnu.

Please also try to build this on Hurd.

> diff --git a/rt/tst-timer-sigmask.c b/rt/tst-timer-sigmask.c
> new file mode 100644
> index 0000000000..1030e4c79f
> --- /dev/null
> +++ b/rt/tst-timer-sigmask.c
> @@ -0,0 +1,96 @@

> +static void
> +thread_handler (union sigval sv)
> +{
> +  bool failure = false;
> +
> +  sigset_t ss;
> +  sigprocmask (SIG_BLOCK, NULL, &ss);
> +  if (test_verbose > 0)
> +    printf ("%s: blocked signal mask = { ", __func__);
> +  for (int sig = 1; sig < NSIG; sig++)
> +    {
> +#ifdef __linux__
> +      /* POSIX timers threads created to handle SIGEV_THREAD blocks
> +	 all signals except SIGKILL, SIGSTOP, and SIGSETXID.  */
> +      if (!sigismember (&ss, sig)
> +	  && (sig != SIGSETXID && sig != SIGKILL && sig != SIGSTOP))
> +	{
> +	  failure = true;
> +	}
> +#endif
> +      if (test_verbose && sigismember (&ss, sig))
> +	printf ("%d, ", sig);
> +    }
> +  if (test_verbose > 0)
> +    printf ("}\n");
> +
> +  pthread_mutex_lock (&lock);
> +  thread_handler_check = failure
> +			 ? THREAD_SIGNAL_CHECK_FAIL : THREAD_SIGNAL_CHECK_OK;
> +  pthread_cond_signal (&cond);
> +  pthread_mutex_unlock (&lock);

Should you use the x variants?  xpthread_sigmask, xpthread_mutex_lock
etc.?

The synchronization might more easily expressed using a POSIX barrier.

> diff --git a/sysdeps/unix/sysv/linux/kernel-posix-timers.h b/sysdeps/unix/sysv/linux/kernel-posix-timers.h
> index 1ded4df51a..d60cb95f80 100644
> --- a/sysdeps/unix/sysv/linux/kernel-posix-timers.h
> +++ b/sysdeps/unix/sysv/linux/kernel-posix-timers.h
> @@ -43,7 +43,6 @@ extern pthread_mutex_t __active_timer_sigev_thread_lock attribute_hidden;
>  /* Type of timers in the kernel.  */
>  typedef int kernel_timer_t;
>  
> -
>  /* Internal representation of timer.  */
>  struct timer
>  {

Unrelated change?

> +		  /* This is the signal we are waiting for.  */
> +		  td->thrfunc = tk->thrfunc;
> +		  td->sival = tk->sival;
>  
> +		  pthread_t th;
> +		  pthread_create (&th, &tk->attr, timer_sigev_thread, td);
> +		}

I think this does not unblock SIGCANCEL on the new thread with the
current pthread_create implementation.  This will change with the
block-signals-around-clone patch I submitted, though.

Thanks,
Florian