Skip to content

Commit

Permalink
imp: linger and forward signals to children of flux-imp run
Browse files Browse the repository at this point in the history
Problem: Unlike `exec`, `flux-imp run` doesn't linger and instead
directly executes the target command. This then requires the use of
`flux-imp kill` to kill the privlieged process(es) that are a result.

Have the IMP fork() and exec() the run command. The lingering IMP
process can then forward signals delivered to it, including the use
of SIGUSR1 as a surrogate for SIGKILL.
  • Loading branch information
grondo committed Nov 1, 2024
1 parent 1e23343 commit 0e39552
Showing 1 changed file with 49 additions and 13 deletions.
62 changes: 49 additions & 13 deletions src/imp/run.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@

#include <sys/types.h>
#include <sys/stat.h>
#include <sys/wait.h>
#include <pwd.h>
#include <signal.h>

Expand All @@ -61,6 +62,7 @@
#include "imp_state.h"
#include "impcmd.h"
#include "privsep.h"
#include "signals.h"

extern char **environ;

Expand Down Expand Up @@ -142,7 +144,8 @@ static struct kv *get_run_env (struct kv *kv, const cf_t *allowed_env)
}

static void __attribute__((noreturn))
imp_run (const char *name,
imp_run (struct imp_state *imp,
const char *name,
const cf_t *run_cf,
struct kv *kv_env)
{
Expand All @@ -151,6 +154,8 @@ imp_run (const char *name,
char **env;
const char *args[2];
int exit_code;
int status;
pid_t child;

if (!(path = cf_string (cf_get_in (run_cf, "path")))
|| path[0] != '/')
Expand Down Expand Up @@ -181,20 +186,51 @@ imp_run (const char *name,
if (chdir ("/") < 0)
imp_die (1, "run: failed to chdir to /");

args[0] = path;
args[1] = NULL;
/* Block signals so parent isn't terminated
*/
imp_sigblock_all ();

if ((child = fork ()) < 0)
imp_die (1, "run: fork: %s", strerror (errno));

imp_set_signal_child (child);

if (child == 0) {
/* unblock all signals */
imp_sigunblock_all ();

args[0] = path;
args[1] = NULL;
#if CODE_COVERAGE_ENABLED
__gcov_dump ();
__gcov_reset ();
__gcov_dump ();
__gcov_reset ();
#endif
execve (path, (char **) args, env);
execve (path, (char **) args, env);

if (errno == EPERM || errno == EACCES)
exit_code = 126;
else
exit_code = 127;
if (errno == EPERM || errno == EACCES)
exit_code = 126;
else
exit_code = 127;
imp_die (exit_code, "%s: %s", path, strerror (errno));
}

/* Parent:
*/
imp_setup_signal_forwarding (imp);

imp_die (exit_code, "%s: %s", path, strerror (errno));
/* Parent: wait for child to exit */
while (waitpid (child, &status, 0) != child) {
if (errno != EINTR)
imp_die (1, "waitpid: %s", strerror (errno));
}

/* Exit with status of the child process */
if (WIFEXITED (status))
exit (WEXITSTATUS (status));
else if (WIFSIGNALED (status))
imp_raise (WTERMSIG (status));
else
exit (1);
}


Expand Down Expand Up @@ -239,7 +275,7 @@ int imp_run_privileged (struct imp_state *imp,
|| setgid (getegid()) < 0)
imp_die (1, "setuid: %s", strerror (errno));

imp_run (name, cf_run, kv_env);
imp_run (imp, name, cf_run, kv_env);

return 0;
}
Expand Down Expand Up @@ -338,7 +374,7 @@ int imp_run_unprivileged (struct imp_state *imp, struct kv *kv)
imp_die (1, "run: permission denied");

kv_env = get_run_env (kv, cf_get_in (cf_run, "allowed-environment"));
imp_run (imp->argv[2], cf_run, kv_env);
imp_run (imp, imp->argv[2], cf_run, kv_env);

return 0;
}
Expand Down

0 comments on commit 0e39552

Please sign in to comment.