From: Yu Watanabe Date: Thu, 22 Sep 2022 04:06:54 +0000 (+0900) Subject: core: do not create symlink to private directory if parent already exists X-Git-Tag: v252-rc2~66^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a2ab603cc42e1484c799f76a233b077c17db91cb;p=thirdparty%2Fsystemd.git core: do not create symlink to private directory if parent already exists The very basic functinality of StateDirectory= or friends is creating specified directories. That should work if one entry is a subdirectory of another. However, it does not when combined with DynamicUser=yes. To support such case, this adds ExecDirectoryItem.only_create flag, and if it is set PID1 only create private directory, and not create the symlink to the private directory. Fixes #24783. --- diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 57036936f9a..fd73c7bcb77 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -3324,6 +3324,7 @@ int bus_exec_context_set_transient_property( if (r < 0) return log_oom(); } + exec_directory_sort(d); joined = unit_concat_strv(l, UNIT_ESCAPE_SPECIFIERS); if (!joined) @@ -3787,6 +3788,8 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; + exec_directory_sort(directory); + r = sd_bus_message_exit_container(message); if (r < 0) return r; diff --git a/src/core/execute.c b/src/core/execute.c index 955e2cf6288..1b75b49426c 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -91,6 +91,7 @@ #include "signal-util.h" #include "smack-util.h" #include "socket-util.h" +#include "sort-util.h" #include "special.h" #include "stat-util.h" #include "string-table.h" @@ -2412,12 +2413,24 @@ static int setup_exec_directory( goto fail; } - /* And link it up from the original place. Note that if a mount namespace is going to be - * used, then this symlink remains on the host, and a new one for the child namespace will - * be created later. */ - r = symlink_idempotent(pp, p, true); - if (r < 0) - goto fail; + if (!context->directories[type].items[i].only_create) { + /* And link it up from the original place. + * Notes + * 1) If a mount namespace is going to be used, then this symlink remains on + * the host, and a new one for the child namespace will be created later. + * 2) It is not necessary to create this symlink when one of its parent + * directories is specified and already created. E.g. + * StateDirectory=foo foo/bar + * In that case, the inode points to pp and p for "foo/bar" are the same: + * pp = "/var/lib/private/foo/bar" + * p = "/var/lib/foo/bar" + * and, /var/lib/foo is a symlink to /var/lib/private/foo. So, not only + * we do not need to create the symlink, but we cannot create the symlink. + * See issue #24783. */ + r = symlink_idempotent(pp, p, true); + if (r < 0) + goto fail; + } } else { _cleanup_free_ char *target = NULL; @@ -3289,7 +3302,8 @@ static int compile_bind_mounts( if (!params->prefix[t]) continue; - n += context->directories[t].n_items; + for (size_t i = 0; i < context->directories[t].n_items; i++) + n += !context->directories[t].items[i].only_create; } if (n <= 0) { @@ -3358,6 +3372,11 @@ static int compile_bind_mounts( for (size_t i = 0; i < context->directories[t].n_items; i++) { char *s, *d; + /* When one of the parent directories is in the list, we cannot create the symlink + * for the child directory. See also the comments in setup_exec_directory(). */ + if (context->directories[t].items[i].only_create) + continue; + if (exec_directory_is_private(context, t)) s = path_join(params->prefix[t], "private", context->directories[t].items[i].path); else @@ -3437,7 +3456,9 @@ static int compile_symlinks( return r; } - if (!exec_directory_is_private(context, dt) || exec_context_with_rootfs(context)) + if (!exec_directory_is_private(context, dt) || + exec_context_with_rootfs(context) || + context->directories[dt].items[i].only_create) continue; private_path = path_join(params->prefix[dt], "private", context->directories[dt].items[i].path); @@ -7090,6 +7111,34 @@ int exec_directory_add(ExecDirectory *d, const char *path, const char *symlink) return 1; /* new item is added */ } +static int exec_directory_item_compare_func(const ExecDirectoryItem *a, const ExecDirectoryItem *b) { + assert(a); + assert(b); + + return path_compare(a->path, b->path); +} + +void exec_directory_sort(ExecDirectory *d) { + assert(d); + + /* Sort the exec directories to make always parent directories processed at first in + * setup_exec_directory(), e.g., even if StateDirectory=foo/bar foo, we need to create foo at first, + * then foo/bar. Also, set .only_create flag if one of the parent directories is contained in the + * list. See also comments in setup_exec_directory() and issue #24783. */ + + if (d->n_items <= 1) + return; + + typesafe_qsort(d->items, d->n_items, exec_directory_item_compare_func); + + for (size_t i = 1; i < d->n_items; i++) + for (size_t j = 0; j < i; j++) + if (path_startswith(d->items[i].path, d->items[j].path)) { + d->items[i].only_create = true; + break; + } +} + DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(exec_set_credential_hash_ops, char, string_hash_func, string_compare_func, ExecSetCredential, exec_set_credential_free); DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(exec_load_credential_hash_ops, char, string_hash_func, string_compare_func, ExecLoadCredential, exec_load_credential_free); diff --git a/src/core/execute.h b/src/core/execute.h index fc6d36aa00d..a2cf22806b2 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -135,6 +135,7 @@ typedef enum ExecDirectoryType { typedef struct ExecDirectoryItem { char *path; char **symlinks; + bool only_create; } ExecDirectoryItem; typedef struct ExecDirectory { @@ -493,6 +494,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(ExecLoadCredential*, exec_load_credential_free); void exec_directory_done(ExecDirectory *d); int exec_directory_add(ExecDirectory *d, const char *path, const char *symlink); +void exec_directory_sort(ExecDirectory *d); extern const struct hash_ops exec_set_credential_hash_ops; extern const struct hash_ops exec_load_credential_hash_ops; diff --git a/src/core/unit.c b/src/core/unit.c index d181d03b7a0..d6bea2080f0 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4131,6 +4131,9 @@ int unit_patch_contexts(Unit *u) { ec->no_new_privileges = true; ec->restrict_suid_sgid = true; } + + for (ExecDirectoryType dt = 0; dt < _EXEC_DIRECTORY_TYPE_MAX; dt++) + exec_directory_sort(ec->directories + dt); } cc = unit_get_cgroup_context(u);