diff --git a/src/imp/run.c b/src/imp/run.c index 06211dd..32e1059 100644 --- a/src/imp/run.c +++ b/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)); diff --git a/src/imp/signals.c b/src/imp/signals.c index 670398c..c4314df 100644 --- a/src/imp/signals.c +++ b/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); } diff --git a/src/imp/signals.h b/src/imp/signals.h index 0bf0208..d9b1896 100644 --- a/src/imp/signals.h +++ b/src/imp/signals.h @@ -14,9 +14,10 @@ #include #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 */ diff --git a/t/t2002-imp-run.t b/t/t2002-imp-run.t index 60f7783..b3c416d 100755 --- a/t/t2002-imp-run.t +++ b/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 &&