bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling

[Pádraig Brady]

On 05/02/17 10:50, Tobias Stoeckmann wrote:
>      {
>        pid_t wait_result;
> +      sigset_t old_set;
>        int status;
>  
> +      /* We configure timers below so that SIGALRM is sent on expiry.
> +         Therefore ensure we don't inherit a mask blocking SIGALRM.  */
> +      unblock_signal (SIGALRM);

cool

>        settimeout (timeout, true);
>  
> -      while ((wait_result = waitpid (monitored_pid, &status, 0)) < 0
> -             && errno == EINTR)
> -        continue;
> +      block_signal (SIGALRM, &old_set);

SIGALRM isn't the only signal causing this race?
If a SIGTERM etc. is sent to the timeout process during the race,
cleanup() will be called with the same consequences.
Don't we need to block all signals for the cleanup() handler?

> +      while ((wait_result = waitpid (monitored_pid, &status, WNOHANG)) == 0)
> +        sigsuspend (&old_set);

> +      monitored_pid = 0;
> +      unblock_signal (SIGALRM);

There doesn't seem to be much point in re-enabling SIGALRM (or other signals) 
here,
as if it does fire it means we'll return 143 rather than the correct 124.
Since the child is reaped, we can just proceed and process the exit status
of the child and exit() I think.

cheers,
Pádraig.