From 70806d455483329ba69917832268800bff411382 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 23 Mar 2023 03:15:43 +0900 Subject: [PATCH 1/5] proc-cmdline: split commandline earlier in proc_cmdline_parse() and friend No functional change, just preparation for later commits. --- src/basic/proc-cmdline.c | 240 ++++++++++++++++++++------------------- 1 file changed, 121 insertions(+), 119 deletions(-) diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index de1f66635a..06822ddb49 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -12,8 +12,8 @@ #include "parse-util.h" #include "proc-cmdline.h" #include "process-util.h" -#include "special.h" #include "string-util.h" +#include "strv.h" #include "virt.h" int proc_cmdline(char **ret) { @@ -40,71 +40,47 @@ int proc_cmdline(char **ret) { return read_one_line_file("/proc/cmdline", ret); } -static int proc_cmdline_extract_first(const char **p, char **ret_word, ProcCmdlineFlags flags) { - const char *q = *p; - int r; - - for (;;) { - _cleanup_free_ char *word = NULL; - const char *c; - - r = extract_first_word(&q, &word, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); - if (r < 0) - return r; - if (r == 0) - break; +static char *mangle_word(const char *word, ProcCmdlineFlags flags) { + char *c; + c = startswith(word, "rd."); + if (c) { /* Filter out arguments that are intended only for the initrd */ - c = startswith(word, "rd."); - if (c) { - if (!in_initrd()) - continue; - if (FLAGS_SET(flags, PROC_CMDLINE_STRIP_RD_PREFIX)) { - r = free_and_strdup(&word, c); - if (r < 0) - return r; - } + if (!in_initrd()) + return NULL; - } else if (FLAGS_SET(flags, PROC_CMDLINE_RD_STRICT) && in_initrd()) - continue; /* And optionally filter out arguments that are intended only for the host */ + if (FLAGS_SET(flags, PROC_CMDLINE_STRIP_RD_PREFIX)) + return c; - *p = q; - *ret_word = TAKE_PTR(word); - return 1; - } + } else if (FLAGS_SET(flags, PROC_CMDLINE_RD_STRICT) && in_initrd()) + /* And optionally filter out arguments that are intended only for the host */ + return NULL; - *p = q; - *ret_word = NULL; - return 0; + return (char*) word; } -static int proc_cmdline_parse_given(const char *line, proc_cmdline_parse_t parse_item, void *data, ProcCmdlineFlags flags) { - const char *p; +static int proc_cmdline_parse_strv(char **args, proc_cmdline_parse_t parse_item, void *data, ProcCmdlineFlags flags) { int r; assert(parse_item); - /* The PROC_CMDLINE_VALUE_OPTIONAL flag doesn't really make sense for proc_cmdline_parse(), let's make this - * clear. */ + /* The PROC_CMDLINE_VALUE_OPTIONAL flag doesn't really make sense for proc_cmdline_parse(), let's + * make this clear. */ assert(!FLAGS_SET(flags, PROC_CMDLINE_VALUE_OPTIONAL)); - p = line; - for (;;) { - _cleanup_free_ char *word = NULL; - char *value; + STRV_FOREACH(word, args) { + char *key, *value; - r = proc_cmdline_extract_first(&p, &word, flags); - if (r < 0) - return r; - if (r == 0) - break; + key = mangle_word(*word, flags); + if (!key) + continue; - value = strchr(word, '='); + value = strchr(key, '='); if (value) - *(value++) = 0; + *(value++) = '\0'; - r = parse_item(word, value, data); + r = parse_item(key, value, data); if (r < 0) return r; } @@ -113,6 +89,7 @@ static int proc_cmdline_parse_given(const char *line, proc_cmdline_parse_t parse } int proc_cmdline_parse(proc_cmdline_parse_t parse_item, void *data, ProcCmdlineFlags flags) { + _cleanup_strv_free_ char **args = NULL; _cleanup_free_ char *line = NULL; int r; @@ -126,10 +103,15 @@ int proc_cmdline_parse(proc_cmdline_parse_t parse_item, void *data, ProcCmdlineF if (r != -ENODATA) log_debug_errno(r, "Failed to get SystemdOptions EFI variable, ignoring: %m"); } else { - r = proc_cmdline_parse_given(line, parse_item, data, flags); + r = strv_split_full(&args, line, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); if (r < 0) return r; + r = proc_cmdline_parse_strv(args, parse_item, data, flags); + if (r < 0) + return r; + + args = strv_free(args); line = mfree(line); } } @@ -138,7 +120,11 @@ int proc_cmdline_parse(proc_cmdline_parse_t parse_item, void *data, ProcCmdlineF if (r < 0) return r; - return proc_cmdline_parse_given(line, parse_item, data, flags); + r = strv_split_full(&args, line, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); + if (r < 0) + return r; + + return proc_cmdline_parse_strv(args, parse_item, data, flags); } static bool relaxed_equal_char(char a, char b) { @@ -173,24 +159,19 @@ bool proc_cmdline_key_streq(const char *x, const char *y) { return true; } -static int cmdline_get_key(const char *line, const char *key, ProcCmdlineFlags flags, char **ret_value) { +static int cmdline_get_key(char **args, const char *key, ProcCmdlineFlags flags, char **ret_value) { _cleanup_free_ char *v = NULL; bool found = false; - const char *p; int r; - assert(line); assert(key); - p = line; - for (;;) { - _cleanup_free_ char *word = NULL; + STRV_FOREACH(p, args) { + const char *word; - r = proc_cmdline_extract_first(&p, &word, flags); - if (r < 0) - return r; - if (r == 0) - break; + word = mangle_word(*p, flags); + if (!word) + continue; if (ret_value) { const char *e; @@ -224,6 +205,7 @@ static int cmdline_get_key(const char *line, const char *key, ProcCmdlineFlags f } int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_value) { + _cleanup_strv_free_ char **args = NULL; _cleanup_free_ char *line = NULL, *v = NULL; int r; @@ -251,10 +233,14 @@ int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_val if (r < 0) return r; - if (FLAGS_SET(flags, PROC_CMDLINE_IGNORE_EFI_OPTIONS)) /* Shortcut */ - return cmdline_get_key(line, key, flags, ret_value); + r = strv_split_full(&args, line, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); + if (r < 0) + return r; - r = cmdline_get_key(line, key, flags, ret_value ? &v : NULL); + if (FLAGS_SET(flags, PROC_CMDLINE_IGNORE_EFI_OPTIONS)) /* Shortcut */ + return cmdline_get_key(args, key, flags, ret_value); + + r = cmdline_get_key(args, key, flags, ret_value ? &v : NULL); if (r < 0) return r; if (r > 0) { @@ -275,7 +261,12 @@ int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_val if (r < 0) return r; - return cmdline_get_key(line, key, flags, ret_value); + args = strv_free(args); + r = strv_split_full(&args, line, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); + if (r < 0) + return r; + + return cmdline_get_key(args, key, flags, ret_value); } int proc_cmdline_get_bool(const char *key, bool *ret) { @@ -303,13 +294,46 @@ int proc_cmdline_get_bool(const char *key, bool *ret) { return 1; } -int proc_cmdline_get_key_many_internal(ProcCmdlineFlags flags, ...) { - _cleanup_free_ char *line = NULL; - bool processing_efi = true; - const char *p; - va_list ap; +static int cmdline_get_key_ap(ProcCmdlineFlags flags, char* const* args, va_list ap) { int r, ret = 0; + for (;;) { + char **v; + const char *k, *e; + + k = va_arg(ap, const char*); + if (!k) + break; + + assert_se(v = va_arg(ap, char**)); + + STRV_FOREACH(p, args) { + const char *word; + + word = mangle_word(*p, flags); + if (!word) + continue; + + e = proc_cmdline_key_startswith(word, k); + if (e && *e == '=') { + r = free_and_strdup(v, e + 1); + if (r < 0) + return r; + + ret++; + } + } + } + + return ret; +} + +int proc_cmdline_get_key_many_internal(ProcCmdlineFlags flags, ...) { + _cleanup_strv_free_ char **args = NULL; + _cleanup_free_ char *line = NULL; + int r, ret = 0; + va_list ap; + /* The PROC_CMDLINE_VALUE_OPTIONAL flag doesn't really make sense for proc_cmdline_get_key_many(), let's make * this clear. */ assert(!FLAGS_SET(flags, PROC_CMDLINE_VALUE_OPTIONAL)); @@ -320,58 +344,36 @@ int proc_cmdline_get_key_many_internal(ProcCmdlineFlags flags, ...) { r = systemd_efi_options_variable(&line); if (r < 0 && r != -ENODATA) log_debug_errno(r, "Failed to get SystemdOptions EFI variable, ignoring: %m"); + if (r >= 0) { + r = strv_split_full(&args, line, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); + if (r < 0) + return r; + + va_start(ap, flags); + r = cmdline_get_key_ap(flags, args, ap); + va_end(ap); + if (r < 0) + return r; + + ret = r; + args = strv_free(args); + line = mfree(line); + } } - p = line; - for (;;) { - _cleanup_free_ char *word = NULL; + r = proc_cmdline(&line); + if (r < 0) + return r; - r = proc_cmdline_extract_first(&p, &word, flags); - if (r < 0) - return r; - if (r == 0) { - /* We finished with this command line. If this was the EFI one, then let's proceed with the regular one */ - if (processing_efi) { - processing_efi = false; + r = strv_split_full(&args, line, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); + if (r < 0) + return r; - line = mfree(line); - r = proc_cmdline(&line); - if (r < 0) - return r; + va_start(ap, flags); + r = cmdline_get_key_ap(flags, args, ap); + va_end(ap); + if (r < 0) + return r; - p = line; - continue; - } - - break; - } - - va_start(ap, flags); - - for (;;) { - char **v; - const char *k, *e; - - k = va_arg(ap, const char*); - if (!k) - break; - - assert_se(v = va_arg(ap, char**)); - - e = proc_cmdline_key_startswith(word, k); - if (e && *e == '=') { - r = free_and_strdup(v, e + 1); - if (r < 0) { - va_end(ap); - return r; - } - - ret++; - } - } - - va_end(ap); - } - - return ret; + return ret + r; } From 94e0130ab0e22866033fb70891bd9155de1111a1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 23 Mar 2023 08:33:59 +0900 Subject: [PATCH 2/5] proc-cmdline: introduce proc_cmdline_strv() When we are running in a container, we parse the command line of PID1 in proc_cmdline_parse() or friends. Previously, first we merge the command line nulstr as a single string, and then split by using extract_first_word(). That's not only redundant, but also unsafe when the command line argument contain a space. This drops the redundant steps, hence we can safely parse arguments with space. --- src/basic/proc-cmdline.c | 51 ++++++++++++++++++++++-------------- src/basic/proc-cmdline.h | 1 + src/test/test-proc-cmdline.c | 8 ++++++ 3 files changed, 40 insertions(+), 20 deletions(-) diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index 06822ddb49..010bb762c4 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -40,6 +40,30 @@ int proc_cmdline(char **ret) { return read_one_line_file("/proc/cmdline", ret); } +int proc_cmdline_strv(char ***ret) { + const char *e; + int r; + + assert(ret); + + /* For testing purposes it is sometimes useful to be able to override what we consider /proc/cmdline to be */ + e = secure_getenv("SYSTEMD_PROC_CMDLINE"); + if (e) + return strv_split_full(ret, e, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); + + if (detect_container() > 0) + return get_process_cmdline_strv(1, /* flags = */ 0, ret); + else { + _cleanup_free_ char *s = NULL; + + r = read_one_line_file("/proc/cmdline", &s); + if (r < 0) + return r; + + return strv_split_full(ret, s, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); + } +} + static char *mangle_word(const char *word, ProcCmdlineFlags flags) { char *c; @@ -90,7 +114,6 @@ static int proc_cmdline_parse_strv(char **args, proc_cmdline_parse_t parse_item, int proc_cmdline_parse(proc_cmdline_parse_t parse_item, void *data, ProcCmdlineFlags flags) { _cleanup_strv_free_ char **args = NULL; - _cleanup_free_ char *line = NULL; int r; assert(parse_item); @@ -98,6 +121,8 @@ int proc_cmdline_parse(proc_cmdline_parse_t parse_item, void *data, ProcCmdlineF /* We parse the EFI variable first, because later settings have higher priority. */ if (!FLAGS_SET(flags, PROC_CMDLINE_IGNORE_EFI_OPTIONS)) { + _cleanup_free_ char *line = NULL; + r = systemd_efi_options_variable(&line); if (r < 0) { if (r != -ENODATA) @@ -112,15 +137,10 @@ int proc_cmdline_parse(proc_cmdline_parse_t parse_item, void *data, ProcCmdlineF return r; args = strv_free(args); - line = mfree(line); } } - r = proc_cmdline(&line); - if (r < 0) - return r; - - r = strv_split_full(&args, line, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); + r = proc_cmdline_strv(&args); if (r < 0) return r; @@ -229,11 +249,7 @@ int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_val if (FLAGS_SET(flags, PROC_CMDLINE_VALUE_OPTIONAL) && !ret_value) return -EINVAL; - r = proc_cmdline(&line); - if (r < 0) - return r; - - r = strv_split_full(&args, line, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); + r = proc_cmdline_strv(&args); if (r < 0) return r; @@ -250,7 +266,6 @@ int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_val return r; } - line = mfree(line); r = systemd_efi_options_variable(&line); if (r == -ENODATA) { if (ret_value) @@ -330,7 +345,6 @@ static int cmdline_get_key_ap(ProcCmdlineFlags flags, char* const* args, va_list int proc_cmdline_get_key_many_internal(ProcCmdlineFlags flags, ...) { _cleanup_strv_free_ char **args = NULL; - _cleanup_free_ char *line = NULL; int r, ret = 0; va_list ap; @@ -341,6 +355,8 @@ int proc_cmdline_get_key_many_internal(ProcCmdlineFlags flags, ...) { /* This call may clobber arguments on failure! */ if (!FLAGS_SET(flags, PROC_CMDLINE_IGNORE_EFI_OPTIONS)) { + _cleanup_free_ char *line = NULL; + r = systemd_efi_options_variable(&line); if (r < 0 && r != -ENODATA) log_debug_errno(r, "Failed to get SystemdOptions EFI variable, ignoring: %m"); @@ -357,15 +373,10 @@ int proc_cmdline_get_key_many_internal(ProcCmdlineFlags flags, ...) { ret = r; args = strv_free(args); - line = mfree(line); } } - r = proc_cmdline(&line); - if (r < 0) - return r; - - r = strv_split_full(&args, line, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); + r = proc_cmdline_strv(&args); if (r < 0) return r; diff --git a/src/basic/proc-cmdline.h b/src/basic/proc-cmdline.h index 8650e293ce..d64d7182b8 100644 --- a/src/basic/proc-cmdline.h +++ b/src/basic/proc-cmdline.h @@ -15,6 +15,7 @@ typedef enum ProcCmdlineFlags { typedef int (*proc_cmdline_parse_t)(const char *key, const char *value, void *data); int proc_cmdline(char **ret); +int proc_cmdline_strv(char ***ret); int proc_cmdline_parse(const proc_cmdline_parse_t parse, void *userdata, ProcCmdlineFlags flags); diff --git a/src/test/test-proc-cmdline.c b/src/test/test-proc-cmdline.c index 7f8330cc24..943eb3513c 100644 --- a/src/test/test-proc-cmdline.c +++ b/src/test/test-proc-cmdline.c @@ -9,6 +9,7 @@ #include "proc-cmdline.h" #include "special.h" #include "string-util.h" +#include "strv.h" #include "tests.h" static int obj; @@ -27,6 +28,7 @@ TEST(proc_cmdline_parse) { TEST(proc_cmdline_override) { _cleanup_free_ char *line = NULL, *value = NULL; + _cleanup_strv_free_ char **args = NULL; assert_se(putenv((char*) "SYSTEMD_PROC_CMDLINE=foo_bar=quux wuff-piep=tuet zumm some_arg_with_space='foo bar' and_one_more=\"zzz aaa\"") == 0); assert_se(putenv((char*) "SYSTEMD_EFI_OPTIONS=different") == 0); @@ -35,6 +37,9 @@ TEST(proc_cmdline_override) { assert_se(proc_cmdline(&line) >= 0); assert_se(streq(line, "foo_bar=quux wuff-piep=tuet zumm some_arg_with_space='foo bar' and_one_more=\"zzz aaa\"")); line = mfree(line); + assert_se(proc_cmdline_strv(&args) >= 0); + assert_se(strv_equal(args, STRV_MAKE("foo_bar=quux", "wuff-piep=tuet", "zumm", "some_arg_with_space=foo bar", "and_one_more=zzz aaa"))); + args = strv_free(args); /* Test if parsing makes uses of the override */ assert_se(proc_cmdline_get_key("foo_bar", 0, &value) > 0 && streq_ptr(value, "quux")); @@ -52,6 +57,9 @@ TEST(proc_cmdline_override) { assert_se(proc_cmdline(&line) >= 0); assert_se(streq(line, "hoge")); line = mfree(line); + assert_se(proc_cmdline_strv(&args) >= 0); + assert_se(strv_equal(args, STRV_MAKE("hoge"))); + args = strv_free(args); assert_se(proc_cmdline_get_key("foo_bar", 0, &value) > 0 && streq_ptr(value, "quux")); value = mfree(value); From dd2d3e975e80f5ae3b64bd9c2b63d866415c764e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 23 Mar 2023 01:34:43 +0900 Subject: [PATCH 3/5] condition: use proc_cmdline_strv() --- src/shared/condition.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/shared/condition.c b/src/shared/condition.c index d5fdbbf9e0..e5a80757e0 100644 --- a/src/shared/condition.c +++ b/src/shared/condition.c @@ -106,35 +106,28 @@ Condition* condition_free_list_type(Condition *head, ConditionType type) { } static int condition_test_kernel_command_line(Condition *c, char **env) { - _cleanup_free_ char *line = NULL; + _cleanup_strv_free_ char **args = NULL; int r; assert(c); assert(c->parameter); assert(c->type == CONDITION_KERNEL_COMMAND_LINE); - r = proc_cmdline(&line); + r = proc_cmdline_strv(&args); if (r < 0) return r; bool equal = strchr(c->parameter, '='); - for (const char *p = line;;) { - _cleanup_free_ char *word = NULL; + STRV_FOREACH(word, args) { bool found; - r = extract_first_word(&p, &word, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX); - if (r < 0) - return r; - if (r == 0) - break; - if (equal) - found = streq(word, c->parameter); + found = streq(*word, c->parameter); else { const char *f; - f = startswith(word, c->parameter); + f = startswith(*word, c->parameter); found = f && IN_SET(*f, 0, '='); } From ef9c12b157a50d63e8a8eb710c013d16c2cea319 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 21 Mar 2023 10:05:33 +0900 Subject: [PATCH 4/5] tree-wide: reset optind to 0 when GNU extensions in optstring are used Otherwise, if getopt() and friends are used before parse_argv(), then the GNU extensions may be ignored. This should not change any behavior at least now, as we usually use getopt_long() only once per invocation. But in the next commit, getopt_long() will be used for other arrays, hence this change will become necessary. --- src/activate/activate.c | 3 +++ src/ask-password/ask-password.c | 4 ++++ src/cgls/cgls.c | 3 +++ src/journal/cat.c | 3 +++ src/login/inhibit.c | 3 +++ src/machine/machinectl.c | 4 ++++ src/nspawn/nspawn.c | 3 +++ src/run/run.c | 3 +++ src/shutdown/shutdown.c | 4 ++++ src/udev/udevadm-lock.c | 3 +++ src/udev/udevadm.c | 3 +++ src/userdb/userdbctl.c | 4 ++++ 12 files changed, 40 insertions(+) diff --git a/src/activate/activate.c b/src/activate/activate.c index 4a63970326..1caa30d7d4 100644 --- a/src/activate/activate.c +++ b/src/activate/activate.c @@ -347,6 +347,9 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); + /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() + * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ + optind = 0; while ((c = getopt_long(argc, argv, "+hl:aE:d", options, NULL)) >= 0) switch (c) { case 'h': diff --git a/src/ask-password/ask-password.c b/src/ask-password/ask-password.c index f161e65973..e9c09b7825 100644 --- a/src/ask-password/ask-password.c +++ b/src/ask-password/ask-password.c @@ -108,6 +108,10 @@ static int parse_argv(int argc, char *argv[]) { /* Note the asymmetry: the long option --echo= allows an optional argument, the short option does * not. */ + + /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() + * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ + optind = 0; while ((c = getopt_long(argc, argv, "+hen", options, NULL)) >= 0) switch (c) { diff --git a/src/cgls/cgls.c b/src/cgls/cgls.c index 32780c606a..a34b0aa604 100644 --- a/src/cgls/cgls.c +++ b/src/cgls/cgls.c @@ -93,6 +93,9 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 1); assert(argv); + /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() + * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ + optind = 0; while ((c = getopt_long(argc, argv, "-hkalM:u::xc", options, NULL)) >= 0) switch (c) { diff --git a/src/journal/cat.c b/src/journal/cat.c index 5908758a8f..d3f7785ad3 100644 --- a/src/journal/cat.c +++ b/src/journal/cat.c @@ -75,6 +75,9 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); + /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() + * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ + optind = 0; while ((c = getopt_long(argc, argv, "+ht:p:", options, NULL)) >= 0) switch (c) { diff --git a/src/login/inhibit.c b/src/login/inhibit.c index 7cd2fd3e66..25ba848492 100644 --- a/src/login/inhibit.c +++ b/src/login/inhibit.c @@ -210,6 +210,9 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); + /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() + * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ + optind = 0; while ((c = getopt_long(argc, argv, "+h", options, NULL)) >= 0) switch (c) { diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index 6a29a32bcd..447f1a70b0 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -2720,6 +2720,10 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); + /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() + * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ + optind = 0; + for (;;) { static const char option_string[] = "-hp:als:H:M:qn:o:E:"; diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 982dffd1b8..75349c3b0e 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -815,6 +815,9 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); + /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() + * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ + optind = 0; while ((c = getopt_long(argc, argv, "+hD:u:abL:M:jS:Z:qi:xp:nUE:P", options, NULL)) >= 0) switch (c) { diff --git a/src/run/run.c b/src/run/run.c index 8377c2e8cd..ee4a151a4a 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -242,6 +242,9 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); + /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() + * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ + optind = 0; while ((c = getopt_long(argc, argv, "+hrH:M:E:p:tPqGdSu:", options, NULL)) >= 0) switch (c) { diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index 42111d2772..5dee1b3a92 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -75,6 +75,10 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 1); assert(argv); + /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() + * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ + optind = 0; + /* "-" prevents getopt from permuting argv[] and moving the verb away * from argv[1]. Our interface to initrd promises it'll be there. */ while ((c = getopt_long(argc, argv, "-", options, NULL)) >= 0) diff --git a/src/udev/udevadm-lock.c b/src/udev/udevadm-lock.c index d19e7561f8..6d8a5c336f 100644 --- a/src/udev/udevadm-lock.c +++ b/src/udev/udevadm-lock.c @@ -75,6 +75,9 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); + /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() + * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ + optind = 0; while ((c = getopt_long(argc, argv, arg_print ? "hVd:b:t:p" : "+hVd:b:t:p", options, NULL)) >= 0) switch (c) { diff --git a/src/udev/udevadm.c b/src/udev/udevadm.c index 30a72f2a42..b803f7bb0f 100644 --- a/src/udev/udevadm.c +++ b/src/udev/udevadm.c @@ -62,6 +62,9 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); + /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() + * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ + optind = 0; while ((c = getopt_long(argc, argv, "+dhV", options, NULL)) >= 0) switch (c) { diff --git a/src/userdb/userdbctl.c b/src/userdb/userdbctl.c index eab0c3af15..67675b4b7f 100644 --- a/src/userdb/userdbctl.c +++ b/src/userdb/userdbctl.c @@ -1150,6 +1150,10 @@ static int parse_argv(int argc, char *argv[]) { arg_services = l; } + /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() + * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ + optind = 0; + for (;;) { int c; From 6339d3e6021f31a8a8907c2613f1aaac279fe745 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 19 Mar 2023 15:43:43 +0900 Subject: [PATCH 5/5] proc-cmdline: filter PID1 arguments when we are running in a container Otherwise, PID1 arguments e.g. "--deserialize 16" may be parsed unexpectedly by generators. Fixes the issue reported at https://github.com/systemd/systemd/issues/24452#issuecomment-1475004433. --- src/basic/getopt-defs.h | 75 ++++++++++++++++++++++++++++++ src/basic/proc-cmdline.c | 88 +++++++++++++++++++++++++++++++++--- src/basic/proc-cmdline.h | 2 + src/core/main.c | 60 +++--------------------- src/shutdown/shutdown.c | 19 ++------ src/test/test-proc-cmdline.c | 44 ++++++++++++++++++ 6 files changed, 214 insertions(+), 74 deletions(-) create mode 100644 src/basic/getopt-defs.h diff --git a/src/basic/getopt-defs.h b/src/basic/getopt-defs.h new file mode 100644 index 0000000000..3efeb6df80 --- /dev/null +++ b/src/basic/getopt-defs.h @@ -0,0 +1,75 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include + +#define SYSTEMD_GETOPT_SHORT_OPTIONS "hDbsz:" + +#define COMMON_GETOPT_ARGS \ + ARG_LOG_LEVEL = 0x100, \ + ARG_LOG_TARGET, \ + ARG_LOG_COLOR, \ + ARG_LOG_LOCATION, \ + ARG_LOG_TIME + +#define SYSTEMD_GETOPT_ARGS \ + ARG_UNIT, \ + ARG_SYSTEM, \ + ARG_USER, \ + ARG_TEST, \ + ARG_NO_PAGER, \ + ARG_VERSION, \ + ARG_DUMP_CONFIGURATION_ITEMS, \ + ARG_DUMP_BUS_PROPERTIES, \ + ARG_BUS_INTROSPECT, \ + ARG_DUMP_CORE, \ + ARG_CRASH_CHVT, \ + ARG_CRASH_SHELL, \ + ARG_CRASH_REBOOT, \ + ARG_CONFIRM_SPAWN, \ + ARG_SHOW_STATUS, \ + ARG_DESERIALIZE, \ + ARG_SWITCHED_ROOT, \ + ARG_DEFAULT_STD_OUTPUT, \ + ARG_DEFAULT_STD_ERROR, \ + ARG_MACHINE_ID, \ + ARG_SERVICE_WATCHDOGS + +#define SHUTDOWN_GETOPT_ARGS \ + ARG_EXIT_CODE, \ + ARG_TIMEOUT + +#define COMMON_GETOPT_OPTIONS \ + { "log-level", required_argument, NULL, ARG_LOG_LEVEL }, \ + { "log-target", required_argument, NULL, ARG_LOG_TARGET }, \ + { "log-color", optional_argument, NULL, ARG_LOG_COLOR }, \ + { "log-location", optional_argument, NULL, ARG_LOG_LOCATION }, \ + { "log-time", optional_argument, NULL, ARG_LOG_TIME } + +#define SYSTEMD_GETOPT_OPTIONS \ + { "unit", required_argument, NULL, ARG_UNIT }, \ + { "system", no_argument, NULL, ARG_SYSTEM }, \ + { "user", no_argument, NULL, ARG_USER }, \ + { "test", no_argument, NULL, ARG_TEST }, \ + { "no-pager", no_argument, NULL, ARG_NO_PAGER }, \ + { "help", no_argument, NULL, 'h' }, \ + { "version", no_argument, NULL, ARG_VERSION }, \ + { "dump-configuration-items", no_argument, NULL, ARG_DUMP_CONFIGURATION_ITEMS }, \ + { "dump-bus-properties", no_argument, NULL, ARG_DUMP_BUS_PROPERTIES }, \ + { "bus-introspect", required_argument, NULL, ARG_BUS_INTROSPECT }, \ + { "dump-core", optional_argument, NULL, ARG_DUMP_CORE }, \ + { "crash-chvt", required_argument, NULL, ARG_CRASH_CHVT }, \ + { "crash-shell", optional_argument, NULL, ARG_CRASH_SHELL }, \ + { "crash-reboot", optional_argument, NULL, ARG_CRASH_REBOOT }, \ + { "confirm-spawn", optional_argument, NULL, ARG_CONFIRM_SPAWN }, \ + { "show-status", optional_argument, NULL, ARG_SHOW_STATUS }, \ + { "deserialize", required_argument, NULL, ARG_DESERIALIZE }, \ + { "switched-root", no_argument, NULL, ARG_SWITCHED_ROOT }, \ + { "default-standard-output", required_argument, NULL, ARG_DEFAULT_STD_OUTPUT, }, \ + { "default-standard-error", required_argument, NULL, ARG_DEFAULT_STD_ERROR, }, \ + { "machine-id", required_argument, NULL, ARG_MACHINE_ID }, \ + { "service-watchdogs", required_argument, NULL, ARG_SERVICE_WATCHDOGS } + +#define SHUTDOWN_GETOPT_OPTIONS \ + { "exit-code", required_argument, NULL, ARG_EXIT_CODE }, \ + { "timeout", required_argument, NULL, ARG_TIMEOUT } diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index 010bb762c4..39e9f2c668 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -7,6 +7,7 @@ #include "efivars.h" #include "extract-word.h" #include "fileio.h" +#include "getopt-defs.h" #include "initrd-util.h" #include "macro.h" #include "parse-util.h" @@ -16,6 +17,66 @@ #include "strv.h" #include "virt.h" +int proc_cmdline_filter_pid1_args( + char **argv, /* input, may be reordered by this function. */ + char ***ret) { + + enum { + COMMON_GETOPT_ARGS, + SYSTEMD_GETOPT_ARGS, + SHUTDOWN_GETOPT_ARGS, + }; + + static const struct option options[] = { + COMMON_GETOPT_OPTIONS, + SYSTEMD_GETOPT_OPTIONS, + SHUTDOWN_GETOPT_OPTIONS, + {} + }; + + int saved_optind, saved_opterr, saved_optopt, argc; + char *saved_optarg; + char **filtered; + size_t idx; + + assert(argv); + assert(ret); + + /* Backup global variables. */ + saved_optind = optind; + saved_opterr = opterr; + saved_optopt = optopt; + saved_optarg = optarg; + + /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() + * that checks for GNU extensions in optstring ('-' or '+' at the beginning). Here, we do not use + * the GNU extensions, but might be used previously. Hence, we need to always reset it. */ + optind = 0; + + /* Do not print an error message. */ + opterr = 0; + + /* Filter out all known options. */ + argc = strv_length(argv); + while (getopt_long(argc, argv, SYSTEMD_GETOPT_SHORT_OPTIONS, options, NULL) >= 0) + ; + + idx = optind; + + /* Restore global variables. */ + optind = saved_optind; + opterr = saved_opterr; + optopt = saved_optopt; + optarg = saved_optarg; + + filtered = strv_copy(strv_skip(argv, idx)); + if (!filtered) + return -ENOMEM; + + *ret = filtered; + return 0; +} + int proc_cmdline(char **ret) { const char *e; @@ -40,7 +101,7 @@ int proc_cmdline(char **ret) { return read_one_line_file("/proc/cmdline", ret); } -int proc_cmdline_strv(char ***ret) { +static int proc_cmdline_strv_internal(char ***ret, bool filter_pid1_args) { const char *e; int r; @@ -51,9 +112,20 @@ int proc_cmdline_strv(char ***ret) { if (e) return strv_split_full(ret, e, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX|EXTRACT_RETAIN_ESCAPE); - if (detect_container() > 0) - return get_process_cmdline_strv(1, /* flags = */ 0, ret); - else { + if (detect_container() > 0) { + _cleanup_strv_free_ char **args = NULL; + + r = get_process_cmdline_strv(1, /* flags = */ 0, &args); + if (r < 0) + return r; + + if (filter_pid1_args) + return proc_cmdline_filter_pid1_args(args, ret); + + *ret = TAKE_PTR(args); + return 0; + + } else { _cleanup_free_ char *s = NULL; r = read_one_line_file("/proc/cmdline", &s); @@ -64,6 +136,10 @@ int proc_cmdline_strv(char ***ret) { } } +int proc_cmdline_strv(char ***ret) { + return proc_cmdline_strv_internal(ret, /* filter_pid1_args = */ false); +} + static char *mangle_word(const char *word, ProcCmdlineFlags flags) { char *c; @@ -140,7 +216,7 @@ int proc_cmdline_parse(proc_cmdline_parse_t parse_item, void *data, ProcCmdlineF } } - r = proc_cmdline_strv(&args); + r = proc_cmdline_strv_internal(&args, /* filter_pid1_args = */ true); if (r < 0) return r; @@ -249,7 +325,7 @@ int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_val if (FLAGS_SET(flags, PROC_CMDLINE_VALUE_OPTIONAL) && !ret_value) return -EINVAL; - r = proc_cmdline_strv(&args); + r = proc_cmdline_strv_internal(&args, /* filter_pid1_args = */ true); if (r < 0) return r; diff --git a/src/basic/proc-cmdline.h b/src/basic/proc-cmdline.h index d64d7182b8..a64d7757bc 100644 --- a/src/basic/proc-cmdline.h +++ b/src/basic/proc-cmdline.h @@ -14,6 +14,8 @@ typedef enum ProcCmdlineFlags { typedef int (*proc_cmdline_parse_t)(const char *key, const char *value, void *data); +int proc_cmdline_filter_pid1_args(char **argv, char ***ret); + int proc_cmdline(char **ret); int proc_cmdline_strv(char ***ret); diff --git a/src/core/main.c b/src/core/main.c index 88bef6740c..23ba22f003 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -49,6 +49,7 @@ #include "fileio.h" #include "format-util.h" #include "fs-util.h" +#include "getopt-defs.h" #include "hexdecoct.h" #include "hostname-setup.h" #include "ima-setup.h" @@ -818,62 +819,13 @@ static void set_manager_settings(Manager *m) { static int parse_argv(int argc, char *argv[]) { enum { - ARG_LOG_LEVEL = 0x100, - ARG_LOG_TARGET, - ARG_LOG_COLOR, - ARG_LOG_LOCATION, - ARG_LOG_TIME, - ARG_UNIT, - ARG_SYSTEM, - ARG_USER, - ARG_TEST, - ARG_NO_PAGER, - ARG_VERSION, - ARG_DUMP_CONFIGURATION_ITEMS, - ARG_DUMP_BUS_PROPERTIES, - ARG_BUS_INTROSPECT, - ARG_DUMP_CORE, - ARG_CRASH_CHVT, - ARG_CRASH_SHELL, - ARG_CRASH_REBOOT, - ARG_CONFIRM_SPAWN, - ARG_SHOW_STATUS, - ARG_DESERIALIZE, - ARG_SWITCHED_ROOT, - ARG_DEFAULT_STD_OUTPUT, - ARG_DEFAULT_STD_ERROR, - ARG_MACHINE_ID, - ARG_SERVICE_WATCHDOGS, + COMMON_GETOPT_ARGS, + SYSTEMD_GETOPT_ARGS, }; static const struct option options[] = { - { "log-level", required_argument, NULL, ARG_LOG_LEVEL }, - { "log-target", required_argument, NULL, ARG_LOG_TARGET }, - { "log-color", optional_argument, NULL, ARG_LOG_COLOR }, - { "log-location", optional_argument, NULL, ARG_LOG_LOCATION }, - { "log-time", optional_argument, NULL, ARG_LOG_TIME }, - { "unit", required_argument, NULL, ARG_UNIT }, - { "system", no_argument, NULL, ARG_SYSTEM }, - { "user", no_argument, NULL, ARG_USER }, - { "test", no_argument, NULL, ARG_TEST }, - { "no-pager", no_argument, NULL, ARG_NO_PAGER }, - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, ARG_VERSION }, - { "dump-configuration-items", no_argument, NULL, ARG_DUMP_CONFIGURATION_ITEMS }, - { "dump-bus-properties", no_argument, NULL, ARG_DUMP_BUS_PROPERTIES }, - { "bus-introspect", required_argument, NULL, ARG_BUS_INTROSPECT }, - { "dump-core", optional_argument, NULL, ARG_DUMP_CORE }, - { "crash-chvt", required_argument, NULL, ARG_CRASH_CHVT }, - { "crash-shell", optional_argument, NULL, ARG_CRASH_SHELL }, - { "crash-reboot", optional_argument, NULL, ARG_CRASH_REBOOT }, - { "confirm-spawn", optional_argument, NULL, ARG_CONFIRM_SPAWN }, - { "show-status", optional_argument, NULL, ARG_SHOW_STATUS }, - { "deserialize", required_argument, NULL, ARG_DESERIALIZE }, - { "switched-root", no_argument, NULL, ARG_SWITCHED_ROOT }, - { "default-standard-output", required_argument, NULL, ARG_DEFAULT_STD_OUTPUT, }, - { "default-standard-error", required_argument, NULL, ARG_DEFAULT_STD_ERROR, }, - { "machine-id", required_argument, NULL, ARG_MACHINE_ID }, - { "service-watchdogs", required_argument, NULL, ARG_SERVICE_WATCHDOGS }, + COMMON_GETOPT_OPTIONS, + SYSTEMD_GETOPT_OPTIONS, {} }; @@ -886,7 +838,7 @@ static int parse_argv(int argc, char *argv[]) { if (getpid_cached() == 1) opterr = 0; - while ((c = getopt_long(argc, argv, "hDbsz:", options, NULL)) >= 0) + while ((c = getopt_long(argc, argv, SYSTEMD_GETOPT_SHORT_OPTIONS, options, NULL)) >= 0) switch (c) { diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index 5dee1b3a92..a8248901ce 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -25,6 +25,7 @@ #include "exec-util.h" #include "fd-util.h" #include "fileio.h" +#include "getopt-defs.h" #include "initrd-util.h" #include "killall.h" #include "log.h" @@ -50,23 +51,13 @@ static usec_t arg_timeout = DEFAULT_TIMEOUT_USEC; static int parse_argv(int argc, char *argv[]) { enum { - ARG_LOG_LEVEL = 0x100, - ARG_LOG_TARGET, - ARG_LOG_COLOR, - ARG_LOG_LOCATION, - ARG_LOG_TIME, - ARG_EXIT_CODE, - ARG_TIMEOUT, + COMMON_GETOPT_ARGS, + SHUTDOWN_GETOPT_ARGS, }; static const struct option options[] = { - { "log-level", required_argument, NULL, ARG_LOG_LEVEL }, - { "log-target", required_argument, NULL, ARG_LOG_TARGET }, - { "log-color", optional_argument, NULL, ARG_LOG_COLOR }, - { "log-location", optional_argument, NULL, ARG_LOG_LOCATION }, - { "log-time", optional_argument, NULL, ARG_LOG_TIME }, - { "exit-code", required_argument, NULL, ARG_EXIT_CODE }, - { "timeout", required_argument, NULL, ARG_TIMEOUT }, + COMMON_GETOPT_OPTIONS, + SHUTDOWN_GETOPT_OPTIONS, {} }; diff --git a/src/test/test-proc-cmdline.c b/src/test/test-proc-cmdline.c index 943eb3513c..6df2bca787 100644 --- a/src/test/test-proc-cmdline.c +++ b/src/test/test-proc-cmdline.c @@ -6,7 +6,9 @@ #include "initrd-util.h" #include "log.h" #include "macro.h" +#include "nulstr-util.h" #include "proc-cmdline.h" +#include "process-util.h" #include "special.h" #include "string-util.h" #include "strv.h" @@ -264,6 +266,48 @@ TEST(proc_cmdline_key_startswith) { assert_se(!proc_cmdline_key_startswith("foo-bar", "foo_xx")); } +#define test_proc_cmdline_filter_pid1_args_one(nulstr, expected) \ + ({ \ + _cleanup_strv_free_ char **a = NULL, **b = NULL; \ + const char s[] = (nulstr); \ + \ + /* This emulates get_process_cmdline_strv(). */ \ + assert_se(a = strv_parse_nulstr_full(s, ELEMENTSOF(s), \ + /* drop_trailing_nuls = */ true)); \ + assert_se(proc_cmdline_filter_pid1_args(a, &b) >= 0); \ + assert_se(strv_equal(b, expected)); \ + }) + +TEST(proc_cmdline_filter_pid1_args) { + test_proc_cmdline_filter_pid1_args_one("systemd\0", + STRV_MAKE_EMPTY); + + test_proc_cmdline_filter_pid1_args_one("systemd\0" + "hoge\0" + "-x\0" + "foo\0" + "--aaa\0" + "var\0", + STRV_MAKE("hoge", "foo", "var")); + + test_proc_cmdline_filter_pid1_args_one("/usr/lib/systemd/systemd\0" + "--switched-root\0" + "--system\0" + "--deserialize\030\0" /* followed with space */ + "--deserialize=31\0" /* followed with '=' */ + "--exit-code=42\0" + "\0\0\0" + "systemd.log_level=debug\0" + "--unit\0foo.target\0" + " ' quoted '\0" + "systemd.log_target=console\0" + "\t\0" + " arg with space \0" + "3\0" + "\0\0\0", + STRV_MAKE("", "", "", "systemd.log_level=debug", " ' quoted '", "systemd.log_target=console", "\t", " arg with space ", "3")); +} + static int intro(void) { if (access("/proc/cmdline", R_OK) < 0 && ERRNO_IS_PRIVILEGE(errno)) return log_tests_skipped("can't read /proc/cmdline");