From 36fea15565e6e5a52469c9702f266c5200b313e4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Mar 2019 10:46:54 +0100 Subject: [PATCH 1/8] util: introduce save_argc_argv() helper --- src/basic/util.h | 5 +++++ src/core/main.c | 3 +-- src/nspawn/nspawn.c | 3 +-- src/test/test-process-util.c | 3 +-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/basic/util.h b/src/basic/util.h index 0d40db059ab..412563fa8e7 100644 --- a/src/basic/util.h +++ b/src/basic/util.h @@ -46,6 +46,11 @@ static inline const char* enable_disable(bool b) { extern int saved_argc; extern char **saved_argv; +static inline void save_argc_argv(int argc, char **argv) { + saved_argc = argc; + saved_argv = argv; +} + bool kexec_loaded(void); int prot_from_flags(int flags) _const_; diff --git a/src/core/main.c b/src/core/main.c index 981513bbc3b..6f6f4e5c5e5 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2380,8 +2380,7 @@ int main(int argc, char *argv[]) { (void) prctl(PR_SET_NAME, systemd); /* Save the original command line */ - saved_argv = argv; - saved_argc = argc; + save_argc_argv(argc, argv); /* Make sure that if the user says "syslog" we actually log to the journal. */ log_set_upgrade_syslog_to_journal(true); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index f3842f70c65..19eb5b6016a 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4668,8 +4668,7 @@ static int run(int argc, char *argv[]) { log_open(); /* Make sure rename_process() in the stub init process can work */ - saved_argv = argv; - saved_argc = argc; + save_argc_argv(argc, argv); r = parse_argv(argc, argv); if (r <= 0) diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c index b5ba651d893..89f6618e2e8 100644 --- a/src/test/test-process-util.c +++ b/src/test/test-process-util.c @@ -603,8 +603,7 @@ static void test_ioprio_class_from_to_string(void) { int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); - saved_argc = argc; - saved_argv = argv; + save_argc_argv(argc, argv); if (argc > 1) { pid_t pid = 0; From 60ffa37a65a96c3af857a3dfc4a6fd47b20cc90e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Mar 2019 14:49:43 +0100 Subject: [PATCH 2/8] main-func: implicitly save argc/argv in DEFINE_MAIN_FUNCTION() functions Let's remove the risk of forgetting to save argc/argv if DEFINE_MAIN_FUNCTION() is used. --- src/nspawn/nspawn.c | 3 --- src/shared/main-func.h | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 19eb5b6016a..dfc4f68ede4 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4667,9 +4667,6 @@ static int run(int argc, char *argv[]) { log_parse_environment(); log_open(); - /* Make sure rename_process() in the stub init process can work */ - save_argc_argv(argc, argv); - r = parse_argv(argc, argv); if (r <= 0) goto finish; diff --git a/src/shared/main-func.h b/src/shared/main-func.h index 3c182e802b9..486976f65e3 100644 --- a/src/shared/main-func.h +++ b/src/shared/main-func.h @@ -8,10 +8,12 @@ #include "spawn-ask-password-agent.h" #include "spawn-polkit-agent.h" #include "static-destruct.h" +#include "util.h" #define _DEFINE_MAIN_FUNCTION(intro, impl, ret) \ int main(int argc, char *argv[]) { \ int r; \ + save_argc_argv(argc, argv); \ intro; \ r = impl; \ static_destruct(); \ From d86e4c979a45ffacf81dcc3c30e748ef0c491c63 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Mar 2019 10:47:19 +0100 Subject: [PATCH 3/8] systemctl: use saved_argv where we can No need to have another variable where we keep the original argv[]. Let's juse reuse the one DEFINE_MAIN_FUNCTION() stores for us anyway. --- src/systemctl/systemctl.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index afe6f84f774..c411873c8bd 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -141,7 +141,6 @@ static const char *arg_kill_who = NULL; static int arg_signal = SIGTERM; static char *arg_root = NULL; static usec_t arg_when = 0; -static char *argv_cmdline = NULL; static enum action { ACTION_SYSTEMCTL, ACTION_HALT, @@ -6177,10 +6176,11 @@ static int switch_root(int argc, char *argv[], void *userdata) { init = NULL; } - /* Instruct PID1 to exclude us from its killing spree applied during - * the transition. Otherwise we would exit with a failure status even - * though the switch to the new root has succeed. */ - argv_cmdline[0] = '@'; + /* Instruct PID1 to exclude us from its killing spree applied during the transition. Otherwise we + * would exit with a failure status even though the switch to the new root has succeed. */ + assert(saved_argv); + assert(saved_argv[0]); + saved_argv[0][0] = '@'; r = acquire_bus(BUS_MANAGER, &bus); if (r < 0) @@ -9161,8 +9161,6 @@ static int logind_cancel_shutdown(void) { static int run(int argc, char *argv[]) { int r; - argv_cmdline = argv[0]; - setlocale(LC_ALL, ""); log_parse_environment(); log_open(); From 3483460cb85eff904e290bd95a3c09409ae22f3a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 21 Mar 2019 17:44:09 +0100 Subject: [PATCH 4/8] systemctl: document argv[] array --- src/systemctl/systemctl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index c411873c8bd..355e8421f50 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -6350,9 +6350,9 @@ static int enable_sysv_units(const char *verb, char **args) { const char *argv[] = { ROOTLIBEXECDIR "/systemd-sysv-install", - NULL, - NULL, - NULL, + NULL, /* --root= */ + NULL, /* verb */ + NULL, /* service */ NULL, }; From b1dffbb91b2accb93962feecf44c4e9b07f06651 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 21 Mar 2019 17:44:30 +0100 Subject: [PATCH 5/8] systemctl: add missing OOM check --- src/systemctl/systemctl.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 355e8421f50..965cbaaf338 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -6397,8 +6397,13 @@ static int enable_sysv_units(const char *verb, char **args) { log_info("%s is not a native service, redirecting to systemd-sysv-install.", name); } - if (!isempty(arg_root)) - argv[c++] = q = strappend("--root=", arg_root); + if (!isempty(arg_root)) { + q = strappend("--root=", arg_root); + if (!q) + return log_oom(); + + argv[c++] = q; + } argv[c++] = verb; argv[c++] = basename(p); From 290cb8e883b2e2ad237967fb2f7d455ecde66b46 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 21 Mar 2019 17:51:08 +0100 Subject: [PATCH 6/8] systemctl: tiny optimization --- src/systemctl/systemctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 965cbaaf338..9d94935d686 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -6449,7 +6449,7 @@ static int enable_sysv_units(const char *verb, char **args) { assert(f > 0); f--; assert(args[f] == name); - strv_remove(args, name); + strv_remove(args + f, name); } #endif From a0c6f3cb662349b317c0dc83ee510ae05b733c64 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 21 Mar 2019 17:57:16 +0100 Subject: [PATCH 7/8] main-func: make sure we destruct memory and stuff last Let's terminate pagers and agents before releasing all memory. --- src/shared/main-func.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shared/main-func.h b/src/shared/main-func.h index 486976f65e3..1b77316d82e 100644 --- a/src/shared/main-func.h +++ b/src/shared/main-func.h @@ -16,11 +16,11 @@ save_argc_argv(argc, argv); \ intro; \ r = impl; \ - static_destruct(); \ ask_password_agent_close(); \ polkit_agent_close(); \ - mac_selinux_finish(); \ pager_close(); \ + mac_selinux_finish(); \ + static_destruct(); \ return ret; \ } From 8d1ee64829966e55904f2dc0e3b2ab928bfcc737 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 21 Mar 2019 18:07:21 +0100 Subject: [PATCH 8/8] systemctl: make a copy of the "verb" from argv[] before forking off a child --- src/systemctl/systemctl.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 9d94935d686..9d2115b7121 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -6356,7 +6356,7 @@ static int enable_sysv_units(const char *verb, char **args) { NULL, }; - _cleanup_free_ char *p = NULL, *q = NULL, *l = NULL; + _cleanup_free_ char *p = NULL, *q = NULL, *l = NULL, *v = NULL; bool found_native = false, found_sysv; const char *name; unsigned c = 1; @@ -6405,7 +6405,13 @@ static int enable_sysv_units(const char *verb, char **args) { argv[c++] = q; } - argv[c++] = verb; + /* Let's copy the verb, since it's still pointing directly into the original argv[] array we + * got passed, but safe_fork() is likely going to rewrite that for the new child */ + v = strdup(verb); + if (!v) + return log_oom(); + + argv[c++] = v; argv[c++] = basename(p); argv[c] = NULL;