From 33d31c0e60502a926e68b653055a4744166c5e3d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 25 Feb 2021 13:16:36 +0100 Subject: [PATCH 1/4] env-util: fix parameter handling of parse_env_extension_hierarchies() + getenv_path_list() Our coding style dictates we should not clobber return parameters on failure, and always initialize them on success. Do so here. This changes getenv_path_list() to return ENXIO if the env var is not set, which is similar to how we handle this in getenv_bool(). This drops debug logging from parse_env_extension_hierarchies(), since it's done anyway in getenv_path_list() Follow-up for: #18018 --- src/basic/env-util.c | 6 ++---- src/shared/extension-release.c | 16 +++++++++------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/basic/env-util.c b/src/basic/env-util.c index 7fa598a3b5..c110a750a5 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -813,11 +813,9 @@ int getenv_path_list(const char *name, char ***ret_paths) { assert(name); assert(ret_paths); - *ret_paths = NULL; - e = secure_getenv(name); if (!e) - return 0; + return -ENXIO; r = strv_split_full(&l, e, ":", EXTRACT_DONT_COALESCE_SEPARATORS); if (r < 0) @@ -842,5 +840,5 @@ int getenv_path_list(const char *name, char ***ret_paths) { "No paths specified, refusing."); *ret_paths = TAKE_PTR(l); - return 0; + return 1; } diff --git a/src/shared/extension-release.c b/src/shared/extension-release.c index 5676e2c063..29cbecbf57 100644 --- a/src/shared/extension-release.c +++ b/src/shared/extension-release.c @@ -79,16 +79,18 @@ int extension_release_validate( } int parse_env_extension_hierarchies(char ***ret_hierarchies) { + _cleanup_free_ char **l = NULL; int r; - r = getenv_path_list("SYSTEMD_SYSEXT_HIERARCHIES", ret_hierarchies); - if (r < 0) - return log_debug_errno(r, "Failed to parse SYSTEMD_SYSEXT_HIERARCHIES environment variable : %m"); - if (!*ret_hierarchies) { - *ret_hierarchies = strv_new("/usr", "/opt"); - if (!*ret_hierarchies) + r = getenv_path_list("SYSTEMD_SYSEXT_HIERARCHIES", &l); + if (r == -ENXIO) { + /* Default when unset */ + l = strv_new("/usr", "/opt"); + if (!l) return -ENOMEM; - } + } else if (r < 0) + return r; + *ret_hierarchies = TAKE_PTR(l); return 0; } From d60e3b405df6aafd27d2072a4e055fb116ab4527 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 25 Feb 2021 13:36:32 +0100 Subject: [PATCH 2/4] namespace: return correct error code --- src/core/namespace.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index ed07db5c73..9a0375fddd 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -1554,9 +1554,12 @@ static int apply_mounts( * /proc. For example, this is the case with the option: 'InaccessiblePaths=/proc'. */ proc_self_mountinfo = fopen("/proc/self/mountinfo", "re"); if (!proc_self_mountinfo) { + r = -errno; + if (error_path) *error_path = strdup("/proc/self/mountinfo"); - return log_debug_errno(errno, "Failed to open /proc/self/mountinfo: %m"); + + return log_debug_errno(r, "Failed to open /proc/self/mountinfo: %m"); } /* First round, establish all mounts we need */ From 2a7865cc431f585d09a35daf186eab7c92e596bb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 25 Feb 2021 13:36:43 +0100 Subject: [PATCH 3/4] bus-unit-util: don't validate partition designator client side When we parse properties set via "systemctl set-property" we should validate to the point where the data we pass over the bus makes rough sense, but we shouldn't needlessly check whether specified enum values are among the known enum values. The server side checks that anyway again, and it's kinda nice if an older systemctl can be used to talk to a newer systemd. --- src/shared/bus-unit-util.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index eaec48fd5e..ce9ae89fe6 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -1631,10 +1631,6 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con return log_error_errno(r, "Failed to parse argument: %m"); STRV_FOREACH_PAIR(first, second, l) { - /* Format is either 'root:foo' or 'foo' (root is implied) */ - if (!isempty(*second) && partition_designator_from_string(*first) < 0) - return bus_log_create_error(-EINVAL); - r = sd_bus_message_append(m, "(ss)", !isempty(*second) ? *first : "root", !isempty(*second) ? *second : *first); @@ -1734,9 +1730,6 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con break; } - if (partition_designator_from_string(partition) < 0) - return bus_log_create_error(-EINVAL); - r = sd_bus_message_append(m, "(ss)", partition, mount_options); if (r < 0) return bus_log_create_error(r); @@ -1838,9 +1831,6 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con break; } - if (partition_designator_from_string(partition) < 0) - return bus_log_create_error(-EINVAL); - r = sd_bus_message_append(m, "(ss)", partition, mount_options); if (r < 0) return bus_log_create_error(r); From de4abc3f4658d76f25fd7404d4003d8bdf093867 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 25 Feb 2021 13:38:20 +0100 Subject: [PATCH 4/4] bus-unit-util: generate proper log message when we fail to parse properties --- src/shared/bus-unit-util.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index ce9ae89fe6..a58495dbf8 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -1679,14 +1679,14 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con r = extract_first_word(&p, &tuple, NULL, EXTRACT_UNQUOTE|EXTRACT_RETAIN_ESCAPE); if (r < 0) - return r; + return log_error_errno(r, "Failed to parse MountImages= property: %s", eq); if (r == 0) break; q = tuple; r = extract_many_words(&q, ":", EXTRACT_CUNESCAPE|EXTRACT_UNESCAPE_SEPARATORS, &first, &second, NULL); if (r < 0) - return r; + return log_error_errno(r, "Failed to parse MountImages= property: %s", eq); if (r == 0) continue; @@ -1718,7 +1718,7 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con r = extract_many_words(&q, ":", EXTRACT_CUNESCAPE|EXTRACT_UNESCAPE_SEPARATORS, &partition, &mount_options, NULL); if (r < 0) - return r; + return log_error_errno(r, "Failed to parse MountImages= property: %s", eq); if (r == 0) break; /* Single set of options, applying to the root partition/single filesystem */ @@ -1785,14 +1785,14 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con r = extract_first_word(&p, &tuple, NULL, EXTRACT_UNQUOTE|EXTRACT_RETAIN_ESCAPE); if (r < 0) - return r; + return log_error_errno(r, "Failed to parse ExtensionImages= property: %s", eq); if (r == 0) break; q = tuple; r = extract_first_word(&q, &source, ":", EXTRACT_CUNESCAPE|EXTRACT_UNESCAPE_SEPARATORS); if (r < 0) - return r; + return log_error_errno(r, "Failed to parse ExtensionImages= property: %s", eq); if (r == 0) continue; @@ -1819,7 +1819,7 @@ static int bus_append_execute_property(sd_bus_message *m, const char *field, con r = extract_many_words(&q, ":", EXTRACT_CUNESCAPE|EXTRACT_UNESCAPE_SEPARATORS, &partition, &mount_options, NULL); if (r < 0) - return r; + return log_error_errno(r, "Failed to parse ExtensionImages= property: %s", eq); if (r == 0) break; /* Single set of options, applying to the root partition/single filesystem */