91 lines
3.3 KiB
Diff
91 lines
3.3 KiB
Diff
From ab4ffc85039f7398dde2ec4b307dfb2aa0fcf4f8 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <P@draigBrady.com>
|
|
Date: Mon, 11 Mar 2024 13:46:24 +0000
|
|
Subject: [PATCH] timeout: fix narrow race in failing to kill processes
|
|
|
|
* src/timeout.c (main): Block cleanup signals earlier so that cleanup()
|
|
is not runnable until monitored_pid is in a deterministic state.
|
|
This ensures we always send a termination signal to the child
|
|
once it's forked.
|
|
* NEWS: Mention the bug fix.
|
|
Reported at https://github.com/coreutils/coreutils/issues/82
|
|
|
|
Reference:https://github.com/coreutils/coreutils/commit/ab4ffc85039f7398dde2ec4b307dfb2aa0fcf4f8
|
|
Conflict:Delete NEWS.
|
|
|
|
---
|
|
src/timeout.c | 32 +++++++++++++++++++++-----------
|
|
2 files changed, 21 insertions(+), 11 deletions(-)
|
|
|
|
diff --git a/src/timeout.c b/src/timeout.c
|
|
index 9aa46a4f5..68d872b12 100644
|
|
--- a/src/timeout.c
|
|
+++ b/src/timeout.c
|
|
@@ -248,7 +248,7 @@ cleanup (int sig)
|
|
{ /* were in the parent, so let it continue to exit below. */
|
|
}
|
|
else /* monitored_pid == 0 */
|
|
- { /* we're the child or the child is not exec'd yet. */
|
|
+ { /* parent hasn't forked yet, or child has not exec'd yet. */
|
|
_exit (128 + sig);
|
|
}
|
|
}
|
|
@@ -537,14 +537,29 @@ main (int argc, char **argv)
|
|
signal (SIGTTOU, SIG_IGN); /* Don't stop if background child needs tty. */
|
|
install_sigchld (); /* Interrupt sigsuspend() when child exits. */
|
|
|
|
+ /* We configure timers so that SIGALRM is sent on expiry.
|
|
+ Therefore ensure we don't inherit a mask blocking SIGALRM. */
|
|
+ unblock_signal (SIGALRM);
|
|
+
|
|
+ /* Block signals now, so monitored_pid is deterministic in cleanup(). */
|
|
+ sigset_t orig_set;
|
|
+ block_cleanup_and_chld (term_signal, &orig_set);
|
|
+
|
|
monitored_pid = fork ();
|
|
if (monitored_pid == -1)
|
|
{
|
|
error (0, errno, _("fork system call failed"));
|
|
return EXIT_CANCELED;
|
|
}
|
|
- else if (monitored_pid == 0)
|
|
- { /* child */
|
|
+ else if (monitored_pid == 0) /* child */
|
|
+ {
|
|
+ /* Restore signal mask for child. */
|
|
+ if (sigprocmask (SIG_SETMASK, &orig_set, nullptr) != 0)
|
|
+ {
|
|
+ error (0, errno, _("child failed to reset signal mask"));
|
|
+ return EXIT_CANCELED;
|
|
+ }
|
|
+
|
|
/* exec doesn't reset SIG_IGN -> SIG_DFL. */
|
|
signal (SIGTTIN, SIG_DFL);
|
|
signal (SIGTTOU, SIG_DFL);
|
|
@@ -561,19 +576,14 @@ main (int argc, char **argv)
|
|
pid_t wait_result;
|
|
int status;
|
|
|
|
- /* We configure timers so that SIGALRM is sent on expiry.
|
|
- Therefore ensure we don't inherit a mask blocking SIGALRM. */
|
|
- unblock_signal (SIGALRM);
|
|
-
|
|
settimeout (timeout, true);
|
|
|
|
- /* Ensure we don't cleanup() after waitpid() reaps the child,
|
|
+ /* Note signals remain blocked in parent here, to 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_and_chld (term_signal, &cleanup_set);
|
|
|
|
while ((wait_result = waitpid (monitored_pid, &status, WNOHANG)) == 0)
|
|
- sigsuspend (&cleanup_set); /* Wait with cleanup signals unblocked. */
|
|
+ sigsuspend (&orig_set); /* Wait with cleanup signals unblocked. */
|
|
|
|
if (wait_result < 0)
|
|
{
|
|
--
|
|
2.43.0
|
|
|