From 1b2710f4d4948a8ee6332894d886d84f79c0587a Mon Sep 17 00:00:00 2001 From: smitsohu Date: Sat, 11 Sep 2021 18:37:03 +0200 Subject: [PATCH] rework exitcodes * add 128 to exitcode if child receives a fatal signal (this is similar to what bash and other shells do) * unify exitcodes across firejail: treat join'ed processes the same as processes in the primary process tree --- src/firejail/join.c | 11 ++++++----- src/firejail/main.c | 13 ++++++++----- src/firejail/sandbox.c | 17 ++++++++++------- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/firejail/join.c b/src/firejail/join.c index 394bbb52891..99fbfdd0a5a 100644 --- a/src/firejail/join.c +++ b/src/firejail/join.c @@ -45,7 +45,7 @@ static unsigned display = 0; static void signal_handler(int sig){ flush_stdin(); - exit(sig); + exit(128 + sig); } static void install_handler(void) { @@ -536,7 +536,6 @@ void join(pid_t pid, int argc, char **argv, int index) { prctl(PR_SET_PDEATHSIG, SIGKILL, 0, 0, 0); #ifdef HAVE_APPARMOR - // add apparmor confinement after the execve set_apparmor(); #endif @@ -596,15 +595,17 @@ void join(pid_t pid, int argc, char **argv, int index) { // end of signal-safe code //***************************** - flush_stdin(); if (WIFEXITED(status)) { + // if we had a proper exit, return that exit status status = WEXITSTATUS(status); } else if (WIFSIGNALED(status)) { - status = WTERMSIG(status); + // distinguish fatal signals by adding 128 + status = 128 + WTERMSIG(status); } else { - status = 0; + status = -1; } + flush_stdin(); exit(status); } diff --git a/src/firejail/main.c b/src/firejail/main.c index e0bf44f62fa..2a9cb7c081a 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -189,13 +189,15 @@ static void my_handler(int s) { logsignal(s); if (waitpid(child, NULL, WNOHANG) == 0) { - if (has_handler(child, s)) // signals are not delivered if there is no handler yet + // child is pid 1 of a pid namespace: + // signals are not delivered if there is no handler yet + if (has_handler(child, s)) kill(child, s); else kill(child, SIGKILL); waitpid(child, NULL, 0); } - myexit(s); + myexit(128 + s); } static void install_handler(void) { @@ -3216,10 +3218,11 @@ printf("link #%s#\n", prf->link); if (WIFEXITED(status)){ myexit(WEXITSTATUS(status)); } else if (WIFSIGNALED(status)) { - myexit(WTERMSIG(status)); + // distinguish fatal signals by adding 128 + myexit(128 + WTERMSIG(status)); } else { - myexit(0); + myexit(1); } - return 0; + return 1; } diff --git a/src/firejail/sandbox.c b/src/firejail/sandbox.c index 59ddfb85585..995827fb78b 100644 --- a/src/firejail/sandbox.c +++ b/src/firejail/sandbox.c @@ -87,9 +87,9 @@ static void sandbox_handler(int sig){ // broadcast a SIGKILL kill(-1, SIGKILL); - flush_stdin(); - exit(sig); + flush_stdin(); + exit(128 + sig); } static void install_handler(void) { @@ -1243,7 +1243,6 @@ int sandbox(void* sandbox_arg) { if (app_pid == 0) { #ifdef HAVE_APPARMOR - // add apparmor confinement after the execve set_apparmor(); #endif @@ -1258,13 +1257,17 @@ int sandbox(void* sandbox_arg) { munmap(set_sandbox_status, 1); int status = monitor_application(app_pid); // monitor application - flush_stdin(); if (WIFEXITED(status)) { // if we had a proper exit, return that exit status - return WEXITSTATUS(status); + status = WEXITSTATUS(status); + } else if (WIFSIGNALED(status)) { + // distinguish fatal signals by adding 128 + status = 128 + WTERMSIG(status); } else { - // something else went wrong! - return -1; + status = -1; } + + flush_stdin(); + return status; }