GNU bug report logs - #25624
[PATCH] timeout: Fix signal race in SIGALRM handling

Previous Next

Full log

Message #20 received at 25624 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Tobias Stoeckmann <tobias <at> stoeckmann.org>
Cc: 25624 <at> debbugs.gnu.org
Subject: Re: bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling
Date: Sun, 5 Feb 2017 18:13:47 -0800

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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.