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

Previous Next

Full log

Message #8 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>, 25624 <at> debbugs.gnu.org
Subject: Re: bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling
Date: Sun, 5 Feb 2017 09:56:54 -0800

On 05/02/17 04:23, Tobias Stoeckmann wrote:
> It is possible to trigger a signal race in timeout if the alarm is
> triggered AFTER the child process already finished and was waited for
> by the parent.


> If a child terminates, it becomes a zombie process until the parent
> called waitpid() (or an equivalent system call) on it. This means that
> the process id cannot ge given to a new process. But if waitpid() has
> been called, the process id stored in the variable monitored_pid could
> already be assigned to another one. If the alarm -- due to timeout --
> is triggered afterwards, the tool can send a signal to that new process
> instead.
> 
> To fix this, I have introduced a critical section around waitpid(),
> which blocks SIGALRM and resets monitored_pid to 0 if waitpid() returned
> anything except 0 (be it a success or a failure). This way, a later
> alarm won't send any signal.

> diff --git a/src/timeout.c b/src/timeout.c
>  settimeout (double duration, bool warn)
>  {
>  
> -  /* We configure timers below so that SIGALRM is sent on expiry.
> -     Therefore ensure we don't inherit a mask blocking SIGALRM.  */
> -  unblock_signal (SIGALRM);

Removing the call above causes this test to fail:
tests/misc/timeout-blocked.pl
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=blob;f=tests/misc/timeout-blocked.pl;hb=HEAD

> -      while ((wait_result = waitpid (monitored_pid, &status, 0)) < 0
> -             && errno == EINTR)
> -        continue;
> +      block_signal (SIGALRM, &old_set);
> +      while ((wait_result = waitpid (monitored_pid, &status, WNOHANG)) == 0)
> +        sigsuspend (&old_set);
> +      monitored_pid = 0;
> +      unblock_signal (SIGALRM);

I think we'd need some conditional code to allow
sigsuspend() to compile on mingw, MSVC 9.

In general this is a largely theoretical race right?
I.E. pids would need to be recycled between the waitpid() and exit()?
Just trying to get an idea of the practicality of the issue.

thanks!
Pádraig

Previous Next

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