Merge pull request #195 from grondo/issue#194

forward signals to entire process group in `flux-imp run`

src/imp/run.c

@@ -193,12 +193,18 @@ imp_run (struct imp_state *imp,
     if ((child = fork ()) < 0)
         imp_die (1, "run: fork: %s", strerror (errno));
 
-    imp_set_signal_child (child);
+    imp_set_signal_child (-child);
 
     if (child == 0) {
         /* unblock all signals */
         imp_sigunblock_all ();
 
+        /* Place child in its own process group, so that parent IMP
+         * can signal the pgrp as a whole
+         */
+        if (setpgrp () < 0)
+            imp_die (1, "setpgrp: %s", strerror (errno));
+
         if (setuid (geteuid()) < 0
             || setgid (getegid()) < 0)
             imp_die (1, "setuid: %s", strerror (errno));

src/imp/signals.c

@@ -37,12 +37,34 @@ void imp_sigblock_all (void)
         imp_die (1, "failed to block signals: %s", strerror (errno));
 }
 
+static void reset_ignored_signals (void)
+{
+    struct sigaction sa;
+    int i;
+
+    memset (&sa, 0, sizeof(sa));
+    sa.sa_handler = SIG_DFL;
+    sa.sa_flags = 0;
+    sigemptyset (&sa.sa_mask);
+
+    for (i = 1; i < SIGRTMIN; i++) {
+        /* Note: it is expected that some signals will fail sigaction(2)
+         * here (e.g. SIGKILL). Just ignore these errors.
+         */
+        (void) sigaction (i, &sa, NULL);
+    }
+}
+
 void imp_sigunblock_all (void)
 {
     sigset_t mask;
     sigemptyset (&mask);
     if (sigprocmask (SIG_SETMASK, &mask, NULL) < 0)
         imp_die (1, "failed to unblock signals: %s", strerror (errno));
+
+    /* Also need to reset any signals that may be currently ignored
+     */
+    reset_ignored_signals ();
 }
 
 static void fwd_signal (int signum)
@@ -67,7 +89,7 @@ static void fwd_signal (int signum)
         if (count < 0)
             imp_warn ("Failed to forward SIGKILL: %s", strerror (errno));
     }
-    else if (imp_child > 0)
+    else if (imp_child != -1)
         kill (imp_child, signum);
 }
 

src/imp/signals.h

@@ -14,9 +14,10 @@
 #include <sys/types.h>
 #include "imp_state.h"
 
-/*  Set the target of IMP signal forwarding
+/*  Set the target of IMP signal forwarding. `pid` may be less than -1,
+ *  in which case the entire process group `-pid` will be signaled.
  */
-void imp_set_signal_child (pid_t child);
+void imp_set_signal_child (pid_t pid);
 
 /*  Setup RFC 15 standard IMP signal forwarding
  */

t/t2002-imp-run.t

@@ -186,12 +186,21 @@ test_expect_success SUDO,NO_CHAIN_LINT 'flux-imp run: setuid IMP lingers' '
 	$SUDO $flux_imp run sleep &
 	imp_pid=$! &&
 	wait_for_file $TESTDIR/sleep.pid &&
-	test_when_finished "$SUDO rm -f $pidfile" &&
+	test_when_finished "$SUDO rm -f $TESTDIR/sleep.pid" &&
 	pid=$(cat $TESTDIR/sleep.pid) &&
 	test $(ps --no-header -o comm -p ${pid}) = "flux-imp" &&
 	kill -TERM $pid &&
 	test_expect_code 143 wait $imp_pid
 '
+test_expect_success SUDO,NO_CHAIN_LINT 'flux-imp run: setuid IMP can successfully forward SIGINT' '
+	$SUDO $flux_imp run sleep &
+	imp_pid=$! &&
+	wait_for_file $TESTDIR/sleep.pid &&
+	test_when_finished "$SUDO rm -f $TESTDIR/sleep.pid" &&
+	pid=$(cat $TESTDIR/sleep.pid) &&
+	kill -INT $pid &&
+	test_expect_code 130 wait $imp_pid
+'
 test_expect_success SUDO 'flux-imp run will not run file with bad ownership' '
 	$SUDO chown $USER $TESTDIR/test.sh &&
 	test_must_fail $SUDO $flux_imp run test &&