[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling
From: |
Pádraig Brady |
Subject: |
bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling |
Date: |
Sun, 5 Feb 2017 09:56:54 -0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 |
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
- bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling, Tobias Stoeckmann, 2017/02/05
- bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling,
Pádraig Brady <=
- bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling, Paul Eggert, 2017/02/05
- bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling, Tobias Stoeckmann, 2017/02/05
- bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling, Tobias Stoeckmann, 2017/02/05
- bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling, Pádraig Brady, 2017/02/05
- bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling, Tobias Stoeckmann, 2017/02/09
- bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling, Pádraig Brady, 2017/02/10
- bug#25624: [PATCH] timeout: Fix signal race in SIGALRM handling, Pádraig Brady, 2017/02/10