diff options
author | Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> | 2022-10-12 09:37:16 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-10-12 09:37:16 +0200 |
commit | 9d4cfc7579cd9a4673ab5433bb7babc821f65627 (patch) | |
tree | 6e819762b64f91068dc40dd1811a98622fce53ca /src | |
parent | 9a72e98f0218c4528e33e7aa19ff17c6dc727a57 (diff) | |
parent | f01f70a9a3f3609c0c8bdbaa4b0b4abbb2b43993 (diff) | |
download | systemd-9d4cfc7579cd9a4673ab5433bb7babc821f65627.tar.gz |
Merge pull request #24784 from yuwata/core-exec-directory
core: do not create symlink to private directory if parent already exists
Diffstat (limited to 'src')
-rw-r--r-- | src/core/dbus-execute.c | 22 | ||||
-rw-r--r-- | src/core/execute.c | 100 | ||||
-rw-r--r-- | src/core/execute.h | 4 | ||||
-rw-r--r-- | src/core/load-fragment.c | 10 | ||||
-rw-r--r-- | src/core/unit.c | 3 |
5 files changed, 99 insertions, 40 deletions
diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 7035d9fbf8..fd73c7bcb7 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -3320,10 +3320,11 @@ int bus_exec_context_set_transient_property( _cleanup_free_ char *joined = NULL; STRV_FOREACH(source, l) { - r = exec_directory_add(&d->items, &d->n_items, *source, NULL); + r = exec_directory_add(d, *source, NULL); if (r < 0) return log_oom(); } + exec_directory_sort(d); joined = unit_concat_strv(l, UNIT_ESCAPE_SPECIFIERS); if (!joined) @@ -3765,21 +3766,8 @@ int bus_exec_context_set_transient_property( if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *destination_escaped = NULL, *source_escaped = NULL; - ExecDirectoryItem *item = NULL; - - /* Adding new directories is supported from both *DirectorySymlink methods and the - * older ones, so try to find an existing configuration first and create it if it's - * not there yet. */ - for (size_t j = 0; j < directory->n_items; ++j) - if (path_equal(source, directory->items[j].path)) { - item = &directory->items[j]; - break; - } - - if (item) - r = strv_extend(&item->symlinks, destination); - else - r = exec_directory_add(&directory->items, &directory->n_items, source, STRV_MAKE(destination)); + + r = exec_directory_add(directory, source, destination); if (r < 0) return r; @@ -3800,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 b73ed8945f..86d238b57d 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); @@ -7041,33 +7062,82 @@ void exec_directory_done(ExecDirectory *d) { d->mode = 0755; } -int exec_directory_add(ExecDirectoryItem **d, size_t *n, const char *path, char **symlinks) { +static ExecDirectoryItem *exec_directory_find(ExecDirectory *d, const char *path) { + assert(d); + assert(path); + + for (size_t i = 0; i < d->n_items; i++) + if (path_equal(d->items[i].path, path)) + return &d->items[i]; + + return NULL; +} + +int exec_directory_add(ExecDirectory *d, const char *path, const char *symlink) { _cleanup_strv_free_ char **s = NULL; _cleanup_free_ char *p = NULL; + ExecDirectoryItem *existing; + int r; assert(d); - assert(n); assert(path); + existing = exec_directory_find(d, path); + if (existing) { + r = strv_extend(&existing->symlinks, symlink); + if (r < 0) + return r; + + return 0; /* existing item is updated */ + } + p = strdup(path); if (!p) return -ENOMEM; - if (symlinks) { - s = strv_copy(symlinks); + if (symlink) { + s = strv_new(symlink); if (!s) return -ENOMEM; } - if (!GREEDY_REALLOC(*d, *n + 1)) + if (!GREEDY_REALLOC(d->items, d->n_items + 1)) return -ENOMEM; - (*d)[(*n) ++] = (ExecDirectoryItem) { + d->items[d->n_items++] = (ExecDirectoryItem) { .path = TAKE_PTR(p), .symlinks = TAKE_PTR(s), }; - return 0; + 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); diff --git a/src/core/execute.h b/src/core/execute.h index 904e7943f3..a2cf22806b 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 { @@ -492,7 +493,8 @@ ExecLoadCredential *exec_load_credential_free(ExecLoadCredential *lc); DEFINE_TRIVIAL_CLEANUP_FUNC(ExecLoadCredential*, exec_load_credential_free); void exec_directory_done(ExecDirectory *d); -int exec_directory_add(ExecDirectoryItem **d, size_t *n, const char *path, char **symlinks); +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/load-fragment.c b/src/core/load-fragment.c index a7136c0b2b..b39cb8cdff 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -4629,10 +4629,8 @@ int config_parse_exec_directories( /* For State and Runtime directories we support an optional destination parameter, which * will be used to create a symlink to the source. */ - _cleanup_strv_free_ char **symlinks = NULL; + _cleanup_free_ char *dresolved = NULL; if (!isempty(dest)) { - _cleanup_free_ char *dresolved = NULL; - if (streq(lvalue, "ConfigurationDirectory")) { log_syntax(unit, LOG_WARNING, filename, line, 0, "Destination parameter is not supported for ConfigurationDirectory, ignoring: %s", tuple); @@ -4649,13 +4647,9 @@ int config_parse_exec_directories( r = path_simplify_and_warn(dresolved, PATH_CHECK_RELATIVE, unit, filename, line, lvalue); if (r < 0) continue; - - r = strv_consume(&symlinks, TAKE_PTR(dresolved)); - if (r < 0) - return log_oom(); } - r = exec_directory_add(&ed->items, &ed->n_items, sresolved, symlinks); + r = exec_directory_add(ed, sresolved, dresolved); if (r < 0) return log_oom(); } diff --git a/src/core/unit.c b/src/core/unit.c index d181d03b7a..d6bea2080f 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); |