From cbf35912da66a17a6113d5a434214dd7651f403a Mon Sep 17 00:00:00 2001 From: =?utf8?q?P=C3=A1draig=20Brady?= Date: Mon, 23 Oct 2017 19:58:56 -0700 Subject: [PATCH] timeout: fix a small race that would ignore command exit This fixes a regression from commit v8.26-39-g2f69dba * src/timeout.c (block_cleanup_and_chld): Rename from block_cleanup to indicate we also block SIGCHLD to avoid the race where SIGCHLD fires between waitpid() polling and sigsuspend() waiting for a signal. * NEWS: Mention the fix. --- NEWS | 4 +++- src/timeout.c | 15 +++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index c3128e1451..18922072ab 100644 --- a/NEWS +++ b/NEWS @@ -12,7 +12,9 @@ GNU coreutils NEWS -*- outline -*- [bug introduced in coreutils-8.28] timeout will again notice its managed command exiting, even when - invoked with blocked CHLD signal. + invoked with blocked CHLD signal, or in a narrow window where + this CHLD signal from the exiting child was missed. In each case + timeout would have then waited for the time limit to expire. [bug introduced in coreutils-8.27] ** Build-related diff --git a/src/timeout.c b/src/timeout.c index 9be70e0152..562781943e 100644 --- a/src/timeout.c +++ b/src/timeout.c @@ -366,19 +366,26 @@ install_cleanup (int sigterm) sigaction (sigterm, &sa, NULL); /* user specified termination signal. */ } -/* Blocks all signals which were registered with cleanup - as signal handler. Return original mask in OLD_SET. */ +/* Block all signals which were registered with cleanup() as the signal + handler, so we never kill processes after waitpid() returns. + Also block SIGCHLD to ensure it doesn't fire between + waitpid() polling and sigsuspend() waiting for a signal. + Return original mask in OLD_SET. */ static void -block_cleanup (int sigterm, sigset_t *old_set) +block_cleanup_and_chld (int sigterm, sigset_t *old_set) { sigset_t block_set; sigemptyset (&block_set); + sigaddset (&block_set, SIGALRM); sigaddset (&block_set, SIGINT); sigaddset (&block_set, SIGQUIT); sigaddset (&block_set, SIGHUP); sigaddset (&block_set, SIGTERM); sigaddset (&block_set, sigterm); + + sigaddset (&block_set, SIGCHLD); + if (sigprocmask (SIG_BLOCK, &block_set, old_set) != 0) error (0, errno, _("warning: sigprocmask")); } @@ -508,7 +515,7 @@ main (int argc, char **argv) /* Ensure we don't cleanup() after waitpid() reaps the child, to avoid sending signals to a possibly different process. */ sigset_t cleanup_set; - block_cleanup (term_signal, &cleanup_set); + block_cleanup_and_chld (term_signal, &cleanup_set); while ((wait_result = waitpid (monitored_pid, &status, WNOHANG)) == 0) sigsuspend (&cleanup_set); /* Wait with cleanup signals unblocked. */ -- 2.47.2