From b2fadec6048adb3596f2633cb7fe7a49f5937a18 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 31 Jul 2014 03:28:37 -0400 Subject: [PATCH] Properly report invalid quoted strings $ systemd-analyze verify trailing-g.service [./trailing-g.service:2] Trailing garbage, ignoring. trailing-g.service lacks ExecStart setting. Refusing. Error: org.freedesktop.systemd1.LoadFailed: Unit trailing-g.service failed to load: Invalid argument. Failed to create trailing-g.service/start: Invalid argument --- src/core/device.c | 12 ++++--- src/core/load-fragment.c | 51 ++++++++++++++++++++++++++++- src/core/main.c | 6 ++++ src/journal-remote/journal-remote.c | 8 +++-- src/journal/journald-server.c | 1 + src/locale/localed.c | 18 +++++----- src/resolve/resolved-manager.c | 1 + src/shared/condition-util.c | 2 ++ src/shared/conf-parser.c | 3 ++ src/shared/install.c | 3 ++ src/shared/log.c | 2 ++ src/shared/strv.c | 13 ++++---- src/shared/strv.h | 2 +- src/shared/util.c | 34 +++++++++++-------- src/sysv-generator/sysv-generator.c | 9 +++++ src/test/test-strv.c | 17 +++++++--- src/test/test-util.c | 1 + src/timesync/timesyncd.c | 2 ++ 18 files changed, 144 insertions(+), 41 deletions(-) diff --git a/src/core/device.c b/src/core/device.c index 2c41c7b6f..0f28a1687 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -223,14 +223,13 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) { const char *word, *state; size_t l; int r; + const char *property; assert(u); assert(dev); - wants = udev_device_get_property_value( - dev, - u->manager->running_as == SYSTEMD_USER ? "SYSTEMD_USER_WANTS" : "SYSTEMD_WANTS"); - + property = u->manager->running_as == SYSTEMD_USER ? "SYSTEMD_USER_WANTS" : "SYSTEMD_WANTS"; + wants = udev_device_get_property_value(dev, property); if (!wants) return 0; @@ -249,6 +248,9 @@ static int device_add_udev_wants(Unit *u, struct udev_device *dev) { if (r < 0) return r; } + if (!isempty(state)) + log_warning_unit(u->id, "Property %s on %s has trailing garbage, ignoring.", + property, strna(udev_device_get_syspath(dev))); return 0; } @@ -407,6 +409,8 @@ static int device_process_new_device(Manager *m, struct udev_device *dev) { else log_warning("SYSTEMD_ALIAS for %s is not an absolute path, ignoring: %s", sysfs, e); } + if (!isempty(state)) + log_warning("SYSTEMD_ALIAS for %s has trailing garbage, ignoring.", sysfs); } return 0; diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index b0448e2c4..d60e28322 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -124,6 +124,8 @@ int config_parse_unit_deps(const char* unit, log_syntax(unit, LOG_ERR, filename, line, -r, "Failed to add dependency on %s, ignoring: %s", k, strerror(-r)); } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, "Invalid syntax, ignoring."); return 0; } @@ -269,6 +271,8 @@ int config_parse_unit_path_strv_printf( k = NULL; } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, "Invalid syntax, ignoring."); return 0; } @@ -568,11 +572,17 @@ int config_parse_exec(const char *unit, k = 0; FOREACH_WORD_QUOTED(word, l, rvalue, state) { if (strneq(word, ";", MAX(l, 1U))) - break; + goto found; k++; } + if (!isempty(state)) { + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); + return 0; + } + found: n = new(char*, k + !honour_argv0); if (!n) return log_oom(); @@ -895,6 +905,9 @@ int config_parse_exec_cpu_affinity(const char *unit, CPU_SET_S(cpu, CPU_ALLOC_SIZE(c->cpuset_ncpus), c->cpuset); } + if (!isempty(state)) + log_syntax(unit, LOG_WARNING, filename, line, EINVAL, + "Trailing garbage, ignoring."); return 0; } @@ -977,6 +990,9 @@ int config_parse_exec_secure_bits(const char *unit, return 0; } } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Invalid syntax, garbage at the end, ignoring."); return 0; } @@ -1031,6 +1047,9 @@ int config_parse_bounding_set(const char *unit, sum |= ((uint64_t) 1ULL) << (uint64_t) cap; } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); if (invert) *capability_bounding_set_drop |= sum; @@ -1187,6 +1206,9 @@ int config_parse_exec_mount_flags(const char *unit, return 0; } } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); c->mount_flags = flags; return 0; @@ -1570,6 +1592,9 @@ int config_parse_service_sockets(const char *unit, if (r < 0) return r; } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); return 0; } @@ -1827,6 +1852,9 @@ int config_parse_environ(const char *unit, strv_free(*env); *env = x; } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); return 0; } @@ -2077,6 +2105,9 @@ int config_parse_unit_requires_mounts_for( continue; } } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); return 0; } @@ -2231,6 +2262,9 @@ int config_parse_syscall_filter( } else set_remove(c->syscall_filter, INT_TO_PTR(id + 1)); } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); /* Turn on NNP, but only if it wasn't configured explicitly * before, and only if we are in user mode. */ @@ -2287,6 +2321,9 @@ int config_parse_syscall_archs( if (r < 0) return log_oom(); } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); return 0; } @@ -2397,6 +2434,9 @@ int config_parse_address_families( } else set_remove(c->address_families, INT_TO_PTR(af)); } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); return 0; } @@ -2905,6 +2945,9 @@ int config_parse_runtime_directory( n = NULL; } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); return 0; } @@ -2979,6 +3022,9 @@ int config_parse_set_status( } } } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); return 0; } @@ -3039,6 +3085,9 @@ int config_parse_namespace_path_strv( n = NULL; } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); return 0; } diff --git a/src/core/main.c b/src/core/main.c index fad15c7c3..e9909ded5 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -488,6 +488,9 @@ static int config_parse_cpu_affinity2( CPU_SET_S(cpu, CPU_ALLOC_SIZE(ncpus), c); } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); if (c) { if (sched_setaffinity(0, CPU_ALLOC_SIZE(ncpus), c) < 0) @@ -636,6 +639,9 @@ static int config_parse_join_controllers(const char *unit, arg_join_controllers = t; } } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); return 0; } diff --git a/src/journal-remote/journal-remote.c b/src/journal-remote/journal-remote.c index 3249027b6..bc36efafa 100644 --- a/src/journal-remote/journal-remote.c +++ b/src/journal-remote/journal-remote.c @@ -149,9 +149,11 @@ static int spawn_getter(const char *getter, const char *url) { _cleanup_strv_free_ char **words = NULL; assert(getter); - words = strv_split_quoted(getter); - if (!words) - return log_oom(); + r = strv_split_quoted(&words, getter); + if (r < 0) { + log_error("Failed to split getter option: %s", strerror(-r)); + return r; + } r = strv_extend(&words, url); if (r < 0) { diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index eac0a4ca6..01da38b8d 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -1326,6 +1326,7 @@ static int server_parse_proc_cmdline(Server *s) { } else if (startswith(word, "systemd.journald")) log_warning("Invalid systemd.journald parameter. Ignoring."); } + /* do not warn about state here, since probably systemd already did */ return 0; } diff --git a/src/locale/localed.c b/src/locale/localed.c index d6ffe6752..bce99b8cb 100644 --- a/src/locale/localed.c +++ b/src/locale/localed.c @@ -210,6 +210,7 @@ static int x11_read_data(Context *c) { FILE *f; char line[LINE_MAX]; bool in_section = false; + int r; context_free_x11(c); @@ -229,10 +230,10 @@ static int x11_read_data(Context *c) { if (in_section && first_word(l, "Option")) { char **a; - a = strv_split_quoted(l); - if (!a) { + r = strv_split_quoted(&a, l); + if (r < 0) { fclose(f); - return -ENOMEM; + return r; } if (strv_length(a) == 3) { @@ -256,8 +257,8 @@ static int x11_read_data(Context *c) { } else if (!in_section && first_word(l, "Section")) { char **a; - a = strv_split_quoted(l); - if (!a) { + r = strv_split_quoted(&a, l); + if (r < 0) { fclose(f); return -ENOMEM; } @@ -533,6 +534,7 @@ static int read_next_mapping(FILE *f, unsigned *n, char ***a) { for (;;) { char line[LINE_MAX]; char *l, **b; + int r; errno = 0; if (!fgets(line, sizeof(line), f)) { @@ -549,9 +551,9 @@ static int read_next_mapping(FILE *f, unsigned *n, char ***a) { if (l[0] == 0 || l[0] == '#') continue; - b = strv_split_quoted(l); - if (!b) - return -ENOMEM; + r = strv_split_quoted(&b, l); + if (r < 0) + return r; if (strv_length(b) < 5) { log_error("Invalid line "SYSTEMD_KBD_MODEL_MAP":%u, ignoring.", *n); diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index 995621c91..fecd7ea0c 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -333,6 +333,7 @@ static int parse_dns_server_string(Manager *m, const char *string) { if (r < 0) return r; } + /* do not warn about state here, since probably systemd already did */ return 0; } diff --git a/src/shared/condition-util.c b/src/shared/condition-util.c index f88ddc19e..b52dcc52f 100644 --- a/src/shared/condition-util.c +++ b/src/shared/condition-util.c @@ -114,6 +114,8 @@ bool condition_test_kernel_command_line(Condition *c) { } } + if (!isempty(state)) + log_warning("Trailing garbage and the end of kernel commandline, ignoring."); free(word); free(line); diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c index cd189adfc..439cfc58f 100644 --- a/src/shared/conf-parser.c +++ b/src/shared/conf-parser.c @@ -717,6 +717,9 @@ int config_parse_strv(const char *unit, if (r < 0) return log_oom(); } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); return 0; } diff --git a/src/shared/install.c b/src/shared/install.c index c32d6599a..276ca3ec7 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -966,6 +966,9 @@ static int config_parse_also( if (r < 0) return r; } + if (!isempty(state)) + log_syntax(unit, LOG_ERR, filename, line, EINVAL, + "Trailing garbage, ignoring."); return 0; } diff --git a/src/shared/log.c b/src/shared/log.c index a7c3195f3..078ccdc35 100644 --- a/src/shared/log.c +++ b/src/shared/log.c @@ -880,6 +880,8 @@ void log_parse_environment(void) { break; } } + if (!isempty(state)) + log_warning("Trailing garbage and the end of kernel commandline, ignoring."); } e = secure_getenv("SYSTEMD_LOG_TARGET"); diff --git a/src/shared/strv.c b/src/shared/strv.c index 0ac66b927..6448f3170 100644 --- a/src/shared/strv.c +++ b/src/shared/strv.c @@ -231,7 +231,7 @@ char **strv_split(const char *s, const char *separator) { return r; } -char **strv_split_quoted(const char *s) { +int strv_split_quoted(char ***t, const char *s) { const char *word, *state; size_t l; unsigned n, i; @@ -242,26 +242,27 @@ char **strv_split_quoted(const char *s) { n = 0; FOREACH_WORD_QUOTED(word, l, s, state) n++; - if (*state) + if (!isempty(state)) /* bad syntax */ - return NULL; + return -EINVAL; r = new(char*, n+1); if (!r) - return NULL; + return -ENOMEM; i = 0; FOREACH_WORD_QUOTED(word, l, s, state) { r[i] = cunescape_length(word, l); if (!r[i]) { strv_free(r); - return NULL; + return -ENOMEM; } i++; } r[i] = NULL; - return r; + *t = r; + return 0; } char **strv_split_newlines(const char *s) { diff --git a/src/shared/strv.h b/src/shared/strv.h index 3034073d3..ee55c148a 100644 --- a/src/shared/strv.h +++ b/src/shared/strv.h @@ -62,7 +62,7 @@ static inline bool strv_isempty(char * const *l) { } char **strv_split(const char *s, const char *separator); -char **strv_split_quoted(const char *s); +int strv_split_quoted(char ***t, const char *s); char **strv_split_newlines(const char *s); char *strv_join(char **l, const char *separator); diff --git a/src/shared/util.c b/src/shared/util.c index cb9687cb0..76cee1926 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -3163,12 +3163,13 @@ fail: } char **replace_env_argv(char **argv, char **env) { - char **r, **i; + char **ret, **i; unsigned k = 0, l = 0; l = strv_length(argv); - if (!(r = new(char*, l+1))) + ret = new(char*, l+1); + if (!ret) return NULL; STRV_FOREACH(i, argv) { @@ -3181,10 +3182,12 @@ char **replace_env_argv(char **argv, char **env) { e = strv_env_get(env, *i+1); if (e) { + int r; - if (!(m = strv_split_quoted(e))) { - r[k] = NULL; - strv_free(r); + r = strv_split_quoted(&m, e); + if (r < 0) { + ret[k] = NULL; + strv_free(ret); return NULL; } } else @@ -3193,16 +3196,17 @@ char **replace_env_argv(char **argv, char **env) { q = strv_length(m); l = l + q - 1; - if (!(w = realloc(r, sizeof(char*) * (l+1)))) { - r[k] = NULL; - strv_free(r); + w = realloc(ret, sizeof(char*) * (l+1)); + if (!w) { + ret[k] = NULL; + strv_free(ret); strv_free(m); return NULL; } - r = w; + ret = w; if (m) { - memcpy(r + k, m, q * sizeof(char*)); + memcpy(ret + k, m, q * sizeof(char*)); free(m); } @@ -3211,14 +3215,16 @@ char **replace_env_argv(char **argv, char **env) { } /* If ${FOO} appears as part of a word, replace it by the variable as-is */ - if (!(r[k++] = replace_env(*i, env))) { - strv_free(r); + ret[k] = replace_env(*i, env); + if (!ret[k]) { + strv_free(ret); return NULL; } + k++; } - r[k] = NULL; - return r; + ret[k] = NULL; + return ret; } int fd_columns(int fd) { diff --git a/src/sysv-generator/sysv-generator.c b/src/sysv-generator/sysv-generator.c index cfae1d75b..368d420df 100644 --- a/src/sysv-generator/sysv-generator.c +++ b/src/sysv-generator/sysv-generator.c @@ -494,6 +494,10 @@ static int load_sysv(SysvStub *s) { "[%s:%u] Failed to add LSB Provides name %s, ignoring: %s", s->path, line, m, strerror(-r)); } + if (!isempty(state_)) + log_error_unit(s->name, + "[%s:%u] Trailing garbage in Provides, ignoring.", + s->path, line); } else if (startswith_no_case(t, "Required-Start:") || startswith_no_case(t, "Should-Start:") || @@ -552,6 +556,11 @@ static int load_sysv(SysvStub *s) { "[%s:%u] Failed to add dependency on %s, ignoring: %s", s->path, line, m, strerror(-r)); } + if (!isempty(state_)) + log_error_unit(s->name, + "[%s:%u] Trailing garbage in %*s, ignoring.", + s->path, line, + (int)(strchr(t, ':') - t), t); } else if (startswith_no_case(t, "Description:")) { char *d, *j; diff --git a/src/test/test-strv.c b/src/test/test-strv.c index cdd2a539a..7ba4c366a 100644 --- a/src/test/test-strv.c +++ b/src/test/test-strv.c @@ -141,6 +141,7 @@ static void test_strv_quote_unquote(const char* const *split, const char *quoted _cleanup_free_ char *p; _cleanup_strv_free_ char **s; char **t; + int r; p = strv_join_quoted((char **)split); assert_se(p); @@ -148,7 +149,8 @@ static void test_strv_quote_unquote(const char* const *split, const char *quoted assert_se(p); assert_se(streq(p, quoted)); - s = strv_split_quoted(quoted); + r = strv_split_quoted(&s, quoted); + assert_se(r == 0); assert_se(s); STRV_FOREACH(t, s) { assert_se(*t); @@ -162,8 +164,10 @@ static void test_strv_unquote(const char *quoted, const char **list) { _cleanup_free_ char *j; unsigned i = 0; char **t; + int r; - s = strv_split_quoted(quoted); + r = strv_split_quoted(&s, quoted); + assert_se(r == 0); assert_se(s); j = strv_join(s, " | "); assert(j); @@ -176,10 +180,12 @@ static void test_strv_unquote(const char *quoted, const char **list) { } static void test_invalid_unquote(const char *quoted) { - char **s; + char **s = NULL; + int r; - s = strv_split_quoted(quoted); + r = strv_split_quoted(&s, quoted); assert(s == NULL); + assert(r == -EINVAL); } static void test_strv_split(void) { @@ -441,6 +447,9 @@ int main(int argc, char *argv[]) { test_invalid_unquote("a --b='c \"d e\"'"); test_invalid_unquote("a --b='c \"d e\" '"); test_invalid_unquote("a --b='c \"d e\"garbage"); + test_invalid_unquote("'"); + test_invalid_unquote("\""); + test_invalid_unquote("'x'y"); test_strv_split(); test_strv_split_newlines(); diff --git a/src/test/test-util.c b/src/test/test-util.c index a56b35567..470475aa5 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -358,6 +358,7 @@ static void test_foreach_word_quoted(void) { assert_se(strneq(expected[i++], word, l)); printf("<%s>\n", t); } + assert(isempty(state)); } static void test_default_term_for_tty(void) { diff --git a/src/timesync/timesyncd.c b/src/timesync/timesyncd.c index 8bb79fb5e..1bd8cf561 100644 --- a/src/timesync/timesyncd.c +++ b/src/timesync/timesyncd.c @@ -1001,6 +1001,8 @@ static int manager_add_server_string(Manager *m, const char *string) { if (r < 0) log_error("Failed to add server %s to configuration, ignoring: %s", t, strerror(-r)); } + if (!isempty(state)) + log_warning("Trailing garbage at the end of server list, ignoring."); return 0; } -- 2.30.2