From 9a653235d12a795a8bd6adf6289ea735ccae71af Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Jun 2023 18:55:14 +0200 Subject: [PATCH 01/16] sd-path: add support for XDG_STATE_HOME --- man/sd_path_lookup.xml | 1 + src/libsystemd/sd-path/sd-path.c | 3 +++ src/path/path.c | 2 ++ src/systemd/sd-path.h | 5 ++++- 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/man/sd_path_lookup.xml b/man/sd_path_lookup.xml index eda43960e7..4c1346712b 100644 --- a/man/sd_path_lookup.xml +++ b/man/sd_path_lookup.xml @@ -55,6 +55,7 @@ SD_PATH_USER_CONFIGURATION, SD_PATH_USER_RUNTIME, + SD_PATH_USER_STATE_PRIVATE, SD_PATH_USER_STATE_CACHE, SD_PATH_USER, diff --git a/src/libsystemd/sd-path/sd-path.c b/src/libsystemd/sd-path/sd-path.c index 5207480360..36c4d89e06 100644 --- a/src/libsystemd/sd-path/sd-path.c +++ b/src/libsystemd/sd-path/sd-path.c @@ -281,6 +281,9 @@ static int get_path(uint64_t type, char **buffer, const char **ret) { case SD_PATH_USER_STATE_CACHE: return from_home_dir("XDG_CACHE_HOME", ".cache", buffer, ret); + case SD_PATH_USER_STATE_PRIVATE: + return from_home_dir("XDG_STATE_HOME", ".local/state", buffer, ret); + case SD_PATH_USER: r = get_home_dir(buffer); if (r < 0) diff --git a/src/path/path.c b/src/path/path.c index 5266240247..1daa345ebe 100644 --- a/src/path/path.c +++ b/src/path/path.c @@ -41,6 +41,8 @@ static const char* const path_table[_SD_PATH_MAX] = { [SD_PATH_USER_CONFIGURATION] = "user-configuration", [SD_PATH_USER_RUNTIME] = "user-runtime", [SD_PATH_USER_STATE_CACHE] = "user-state-cache", + [SD_PATH_USER_STATE_PRIVATE] = "user-state-private", + [SD_PATH_USER] = "user", [SD_PATH_USER_DOCUMENTS] = "user-documents", [SD_PATH_USER_MUSIC] = "user-music", diff --git a/src/systemd/sd-path.h b/src/systemd/sd-path.h index 0b61484f5e..d3ee2eff6b 100644 --- a/src/systemd/sd-path.h +++ b/src/systemd/sd-path.h @@ -53,9 +53,10 @@ enum { SD_PATH_USER_SHARED, /* User configuration, state, runtime ... */ - SD_PATH_USER_CONFIGURATION, /* takes both actual configuration (like /etc) and state (like /var/lib) */ + SD_PATH_USER_CONFIGURATION, SD_PATH_USER_RUNTIME, SD_PATH_USER_STATE_CACHE, + /* → SD_PATH_USER_STATE_PRIVATE is added at the bottom */ /* User resources */ SD_PATH_USER, /* $HOME itself */ @@ -116,6 +117,8 @@ enum { SD_PATH_SYSTEMD_SEARCH_SYSTEM_ENVIRONMENT_GENERATOR, SD_PATH_SYSTEMD_SEARCH_USER_ENVIRONMENT_GENERATOR, + SD_PATH_USER_STATE_PRIVATE, + _SD_PATH_MAX }; From 4bbfc9eac53a9bd1d239312e2572ad352e418d20 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Jun 2023 18:55:39 +0200 Subject: [PATCH 02/16] sd-path: bring spacing in sd-path.h and systemd-path tool in sync --- src/path/path.c | 17 +++++++++++++---- src/systemd/sd-path.h | 1 + 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/path/path.c b/src/path/path.c index 1daa345ebe..f7486ec6f8 100644 --- a/src/path/path.c +++ b/src/path/path.c @@ -20,6 +20,7 @@ static const char *arg_suffix = NULL; static const char* const path_table[_SD_PATH_MAX] = { [SD_PATH_TEMPORARY] = "temporary", [SD_PATH_TEMPORARY_LARGE] = "temporary-large", + [SD_PATH_SYSTEM_BINARIES] = "system-binaries", [SD_PATH_SYSTEM_INCLUDE] = "system-include", [SD_PATH_SYSTEM_LIBRARY_PRIVATE] = "system-library-private", @@ -27,6 +28,7 @@ static const char* const path_table[_SD_PATH_MAX] = { [SD_PATH_SYSTEM_SHARED] = "system-shared", [SD_PATH_SYSTEM_CONFIGURATION_FACTORY] = "system-configuration-factory", [SD_PATH_SYSTEM_STATE_FACTORY] = "system-state-factory", + [SD_PATH_SYSTEM_CONFIGURATION] = "system-configuration", [SD_PATH_SYSTEM_RUNTIME] = "system-runtime", [SD_PATH_SYSTEM_RUNTIME_LOGS] = "system-runtime-logs", @@ -34,10 +36,12 @@ static const char* const path_table[_SD_PATH_MAX] = { [SD_PATH_SYSTEM_STATE_LOGS] = "system-state-logs", [SD_PATH_SYSTEM_STATE_CACHE] = "system-state-cache", [SD_PATH_SYSTEM_STATE_SPOOL] = "system-state-spool", + [SD_PATH_USER_BINARIES] = "user-binaries", [SD_PATH_USER_LIBRARY_PRIVATE] = "user-library-private", [SD_PATH_USER_LIBRARY_ARCH] = "user-library-arch", [SD_PATH_USER_SHARED] = "user-shared", + [SD_PATH_USER_CONFIGURATION] = "user-configuration", [SD_PATH_USER_RUNTIME] = "user-runtime", [SD_PATH_USER_STATE_CACHE] = "user-state-cache", @@ -52,6 +56,7 @@ static const char* const path_table[_SD_PATH_MAX] = { [SD_PATH_USER_PUBLIC] = "user-public", [SD_PATH_USER_TEMPLATES] = "user-templates", [SD_PATH_USER_DESKTOP] = "user-desktop", + [SD_PATH_SEARCH_BINARIES] = "search-binaries", [SD_PATH_SEARCH_BINARIES_DEFAULT] = "search-binaries-default", [SD_PATH_SEARCH_LIBRARY_PRIVATE] = "search-library-private", @@ -62,18 +67,22 @@ static const char* const path_table[_SD_PATH_MAX] = { [SD_PATH_SEARCH_CONFIGURATION] = "search-configuration", [SD_PATH_SYSTEMD_UTIL] = "systemd-util", + [SD_PATH_SYSTEMD_SYSTEM_UNIT] = "systemd-system-unit", [SD_PATH_SYSTEMD_SYSTEM_PRESET] = "systemd-system-preset", [SD_PATH_SYSTEMD_SYSTEM_CONF] = "systemd-system-conf", - [SD_PATH_SYSTEMD_SEARCH_SYSTEM_UNIT] = "systemd-search-system-unit", - [SD_PATH_SYSTEMD_SYSTEM_GENERATOR] = "systemd-system-generator", - [SD_PATH_SYSTEMD_SEARCH_SYSTEM_GENERATOR] = "systemd-search-system-generator", [SD_PATH_SYSTEMD_USER_UNIT] = "systemd-user-unit", [SD_PATH_SYSTEMD_USER_PRESET] = "systemd-user-preset", [SD_PATH_SYSTEMD_USER_CONF] = "systemd-user-conf", + + [SD_PATH_SYSTEMD_SEARCH_SYSTEM_UNIT] = "systemd-search-system-unit", [SD_PATH_SYSTEMD_SEARCH_USER_UNIT] = "systemd-search-user-unit", - [SD_PATH_SYSTEMD_SEARCH_USER_GENERATOR] = "systemd-search-user-generator", + + [SD_PATH_SYSTEMD_SYSTEM_GENERATOR] = "systemd-system-generator", [SD_PATH_SYSTEMD_USER_GENERATOR] = "systemd-user-generator", + [SD_PATH_SYSTEMD_SEARCH_SYSTEM_GENERATOR] = "systemd-search-system-generator", + [SD_PATH_SYSTEMD_SEARCH_USER_GENERATOR] = "systemd-search-user-generator", + [SD_PATH_SYSTEMD_SLEEP] = "systemd-sleep", [SD_PATH_SYSTEMD_SHUTDOWN] = "systemd-shutdown", diff --git a/src/systemd/sd-path.h b/src/systemd/sd-path.h index d3ee2eff6b..fcd90aa967 100644 --- a/src/systemd/sd-path.h +++ b/src/systemd/sd-path.h @@ -83,6 +83,7 @@ enum { * replaces "path" by "search"), since this API is about dirs/paths anyway, and contains "path" * already in the prefix */ SD_PATH_SYSTEMD_UTIL, + SD_PATH_SYSTEMD_SYSTEM_UNIT, SD_PATH_SYSTEMD_SYSTEM_PRESET, SD_PATH_SYSTEMD_SYSTEM_CONF, From 17f06e97e4d07448b579086b2e0217f84236d634 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Jun 2023 18:55:52 +0200 Subject: [PATCH 03/16] path tool: add some basic ansi highlighing --- src/path/path.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/path/path.c b/src/path/path.c index f7486ec6f8..3e022a467a 100644 --- a/src/path/path.c +++ b/src/path/path.c @@ -118,7 +118,7 @@ static int list_homes(void) { continue; } - printf("%s: %s\n", path_table[i], p); + printf("%s%s:%s %s\n", ansi_highlight(), path_table[i], ansi_normal(), p); } return r; From 170d978b2f85aa0ea5c994d7821dfbf6870cffb9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Jun 2023 22:34:25 +0200 Subject: [PATCH 04/16] execude: include RuntimeScope field in ExecParameters Let's decouple execute.c a bit from the Manager object, let's pass the runtime scope (i.e. the enum that discern invocation for user or system context) as part of ExecParameters. This makes the scope available in various functions without having to pass the Manager object in. --- src/core/execute.c | 20 +++++++++++++------- src/core/execute.h | 3 +++ src/core/unit.c | 2 ++ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index 90b19b9e9b..bcd761b755 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -4066,7 +4066,7 @@ static int apply_mount_namespace( return -ENOMEM; } - if (MANAGER_IS_SYSTEM(u->manager)) { + if (params->runtime_scope == RUNTIME_SCOPE_SYSTEM) { propagate_dir = path_join("/run/systemd/propagate/", u->id); if (!propagate_dir) return -ENOMEM; @@ -4078,9 +4078,12 @@ static int apply_mount_namespace( extension_dir = strdup("/run/systemd/unit-extensions"); if (!extension_dir) return -ENOMEM; - } else + } else { + assert(params->runtime_scope == RUNTIME_SCOPE_USER); + if (asprintf(&extension_dir, "/run/user/" UID_FMT "/systemd/unit-extensions", geteuid()) < 0) return -ENOMEM; + } if (root_image) { r = verity_settings_prepare( @@ -4707,14 +4710,17 @@ static void log_command_line(Unit *unit, const char *msg, const char *executable LOG_UNIT_INVOCATION_ID(unit)); } -static bool exec_context_need_unprivileged_private_users(const ExecContext *context, const Manager *manager) { +static bool exec_context_need_unprivileged_private_users( + const ExecContext *context, + const ExecParameters *params) { + assert(context); - assert(manager); + assert(params); /* These options require PrivateUsers= when used in user units, as we need to be in a user namespace * to have permission to enable them when not running as root. If we have effective CAP_SYS_ADMIN * (system manager) then we have privileges and don't need this. */ - if (MANAGER_IS_SYSTEM(manager)) + if (params->runtime_scope != RUNTIME_SCOPE_USER) return false; return context->private_users || @@ -4924,7 +4930,7 @@ static int exec_child( * invocations themselves. Also note that while we'll only invoke NSS modules involved in user management they * might internally call into other NSS modules that are involved in hostname resolution, we never know. */ if (setenv("SYSTEMD_ACTIVATION_UNIT", unit->id, true) != 0 || - setenv("SYSTEMD_ACTIVATION_SCOPE", runtime_scope_to_string(unit->manager->runtime_scope), true) != 0) { + setenv("SYSTEMD_ACTIVATION_SCOPE", runtime_scope_to_string(params->runtime_scope), true) != 0) { *exit_status = EXIT_MEMORY; return log_unit_error_errno(unit, errno, "Failed to update environment: %m"); } @@ -5392,7 +5398,7 @@ static int exec_child( } } - if (needs_sandboxing && exec_context_need_unprivileged_private_users(context, unit->manager)) { + if (needs_sandboxing && exec_context_need_unprivileged_private_users(context, params)) { /* If we're unprivileged, set up the user namespace first to enable use of the other namespaces. * Users with CAP_SYS_ADMIN can set up user namespaces last because they will be able to * set up the all of the other namespaces (i.e. network, mount, UTS) without a user namespace. */ diff --git a/src/core/execute.h b/src/core/execute.h index ee73fb6367..f647c430ae 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -27,6 +27,7 @@ typedef struct Manager Manager; #include "numa-util.h" #include "open-file.h" #include "path-util.h" +#include "runtime-scope.h" #include "set.h" #include "time-util.h" @@ -418,6 +419,8 @@ typedef enum ExecFlags { /* Parameters for a specific invocation of a command. This structure is put together right before a command is * executed. */ struct ExecParameters { + RuntimeScope runtime_scope; + char **environment; int *fds; diff --git a/src/core/unit.c b/src/core/unit.c index 41b520563b..81467093e7 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -5309,6 +5309,8 @@ int unit_set_exec_params(Unit *u, ExecParameters *p) { if (r < 0) return r; + p->runtime_scope = u->manager->runtime_scope; + p->confirm_spawn = manager_get_confirm_spawn(u->manager); p->cgroup_supported = u->manager->cgroup_supported; p->prefix = u->manager->prefix; From d9e5137185e53ad7b8ec2ebbf23f0f990e23b4cb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Jun 2023 22:36:31 +0200 Subject: [PATCH 05/16] execute: remove redundant assignment --- src/core/execute.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/execute.h b/src/core/execute.h index f647c430ae..09f007bb4e 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -141,7 +141,7 @@ struct ExecRuntime { }; typedef enum ExecDirectoryType { - EXEC_DIRECTORY_RUNTIME = 0, + EXEC_DIRECTORY_RUNTIME, EXEC_DIRECTORY_STATE, EXEC_DIRECTORY_CACHE, EXEC_DIRECTORY_LOGS, From d5602c16324ec545c82bb59a3d60a349da7c370c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Jun 2023 11:09:03 +0200 Subject: [PATCH 06/16] execute: when recursively chowning StateDirectory= when spawning services, follow initial symlink It should be OK to allow one level of symlink for the various types of directories like StateDirectory=, LogsDirectory= and such. --- src/core/execute.c | 2 +- src/shared/chown-recursive.c | 7 +++++-- src/shared/chown-recursive.h | 2 +- src/test/test-chown-rec.c | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index bcd761b755..11d707b59c 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2689,7 +2689,7 @@ static int setup_exec_directory( /* Then, change the ownership of the whole tree, if necessary. When dynamic users are used we * drop the suid/sgid bits, since we really don't want SUID/SGID files for dynamic UID/GID * assignments to exist. */ - r = path_chown_recursive(pp ?: p, uid, gid, context->dynamic_user ? 01777 : 07777); + r = path_chown_recursive(pp ?: p, uid, gid, context->dynamic_user ? 01777 : 07777, AT_SYMLINK_FOLLOW); if (r < 0) goto fail; } diff --git a/src/shared/chown-recursive.c b/src/shared/chown-recursive.c index 883c1ccee4..6aa5f6723e 100644 --- a/src/shared/chown-recursive.c +++ b/src/shared/chown-recursive.c @@ -111,12 +111,15 @@ int path_chown_recursive( const char *path, uid_t uid, gid_t gid, - mode_t mask) { + mode_t mask, + int flags) { _cleanup_close_ int fd = -EBADF; struct stat st; - fd = open(path, O_RDONLY|O_DIRECTORY|O_CLOEXEC|O_NOFOLLOW|O_NOATIME); + assert((flags & ~AT_SYMLINK_FOLLOW) == 0); + + fd = open(path, O_RDONLY|O_DIRECTORY|O_CLOEXEC|O_NOATIME|(FLAGS_SET(flags, AT_SYMLINK_FOLLOW) ? 0 : O_NOFOLLOW)); if (fd < 0) return -errno; diff --git a/src/shared/chown-recursive.h b/src/shared/chown-recursive.h index 00038c3b32..2aab8e7414 100644 --- a/src/shared/chown-recursive.h +++ b/src/shared/chown-recursive.h @@ -3,6 +3,6 @@ #include -int path_chown_recursive(const char *path, uid_t uid, gid_t gid, mode_t mask); +int path_chown_recursive(const char *path, uid_t uid, gid_t gid, mode_t mask, int flags); int fd_chown_recursive(int fd, uid_t uid, gid_t gid, mode_t mask); diff --git a/src/test/test-chown-rec.c b/src/test/test-chown-rec.c index 801b49f7b7..dcff17efec 100644 --- a/src/test/test-chown-rec.c +++ b/src/test/test-chown-rec.c @@ -104,7 +104,7 @@ TEST(chown_recursive) { assert_se(st.st_gid == gid); assert_se(has_xattr(p)); - assert_se(path_chown_recursive(t, 1, 2, 07777) >= 0); + assert_se(path_chown_recursive(t, 1, 2, 07777, 0) >= 0); p = strjoina(t, "/dir"); assert_se(lstat(p, &st) >= 0); From f9c91932b4d83faf0f95624dc82db353d0726425 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Jun 2023 22:42:33 +0200 Subject: [PATCH 07/16] execute: add support for XDG_STATE_HOME for placing service state data in --user mode This adds support for the new XDG_STATE_HOME env var that was added to the xdg basedir spec. Previously, because the basedir spec didn't know the concept we'd alias the backing dir for StateDirectory= to the one for ConfigurationDirectory= when runnin in --user mode. With this change we'll make separate. This brings us various benefits, such as proper "systemctl clean" support, where we can clear service state separately from service configuration, now in user mode too. This does not come without complications: retaining compatibility with older setups is difficult, because we cannot possibly identitfy which files in existing populated config dirs are actually "state" and which one are true" configuration. Hence let's deal with this pragmatically: if we detect that a service that has both dirs configured only has the configuration dir existing, then symlink the state dir to the configuration dir to retain compatibility. This is not great, but it's the only somewhat reasonable way out I can see. Fixes: #25739 --- man/systemd.exec.xml | 4 +- man/systemd.unit.xml | 4 +- src/core/execute.c | 55 +++++++++++++++++++ src/core/manager.c | 4 +- src/core/unit-printf.c | 4 +- test/test-execute/exec-specifier-user.service | 4 +- 6 files changed, 65 insertions(+), 10 deletions(-) diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index 4752e0e0f3..32128d4fab 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -1378,7 +1378,7 @@ CapabilityBoundingSet=~CAP_B CAP_C StateDirectory= /var/lib/ - $XDG_CONFIG_HOME + $XDG_STATE_HOME $STATE_DIRECTORY @@ -1390,7 +1390,7 @@ CapabilityBoundingSet=~CAP_B CAP_C LogsDirectory= /var/log/ - $XDG_CONFIG_HOME/log/ + $XDG_STATE_HOME/log/ $LOGS_DIRECTORY diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index e9c7cb238c..8c3329995d 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -2131,7 +2131,7 @@ Note that this setting is not influenced by the Us %L Log directory root - This is either /var/log (for the system manager) or the path $XDG_CONFIG_HOME resolves to with /log appended (for user managers). + This is either /var/log (for the system manager) or the path $XDG_STATE_HOME resolves to with /log appended (for user managers). @@ -2171,7 +2171,7 @@ Note that this setting is not influenced by the Us %S State directory root - This is either /var/lib (for the system manager) or the path $XDG_CONFIG_HOME resolves to (for user managers). + This is either /var/lib (for the system manager) or the path $XDG_STATE_HOME resolves to (for user managers). %t diff --git a/src/core/execute.c b/src/core/execute.c index 11d707b59c..3e065b2ca8 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2511,6 +2511,61 @@ static int setup_exec_directory( if (r < 0) goto fail; + if (IN_SET(type, EXEC_DIRECTORY_STATE, EXEC_DIRECTORY_LOGS) && params->runtime_scope == RUNTIME_SCOPE_USER) { + + /* If we are in user mode, and a configuration directory exists but a state directory + * doesn't exist, then we likely are upgrading from an older systemd version that + * didn't know the more recent addition to the xdg-basedir spec: the $XDG_STATE_HOME + * directory. In older systemd versions EXEC_DIRECTORY_STATE was aliased to + * EXEC_DIRECTORY_CONFIGURATION, with the advent of $XDG_STATE_HOME is is now + * seperated. If a service has both dirs configured but only the configuration dir + * exists and the state dir does not, we assume we are looking at an update + * situation. Hence, create a compatibility symlink, so that all expectations are + * met. + * + * (We also do something similar with the log directory, which still doesn't exist in + * the xdg basedir spec. We'll make it a subdir of the state dir.) */ + + /* this assumes the state dir is always created before the configuration dir */ + assert_cc(EXEC_DIRECTORY_STATE < EXEC_DIRECTORY_LOGS); + assert_cc(EXEC_DIRECTORY_LOGS < EXEC_DIRECTORY_CONFIGURATION); + + r = laccess(p, F_OK); + if (r == -ENOENT) { + _cleanup_free_ char *q = NULL; + + /* OK, we know that the state dir does not exist. Let's see if the dir exists + * under the configuration hierarchy. */ + + if (type == EXEC_DIRECTORY_STATE) + q = path_join(params->prefix[EXEC_DIRECTORY_CONFIGURATION], context->directories[type].items[i].path); + else if (type == EXEC_DIRECTORY_LOGS) + q = path_join(params->prefix[EXEC_DIRECTORY_CONFIGURATION], "log", context->directories[type].items[i].path); + else + assert_not_reached(); + if (!q) { + r = -ENOMEM; + goto fail; + } + + r = laccess(q, F_OK); + if (r >= 0) { + /* It does exist! This hence looks like an update. Symlink the + * configuration directory into the state directory. */ + + r = symlink_idempotent(q, p, /* make_relative= */ true); + if (r < 0) + goto fail; + + log_notice("Unit state directory %s missing but matching configuration directory %s exists, assuming update from systemd 253 or older, creating compatibility symlink.", p, q); + continue; + } else if (r != -ENOENT) + log_warning_errno(r, "Unable to detect whether unit configuration directory '%s' exists, assuming not: %m", q); + + } else if (r < 0) + log_warning_errno(r, "Unable to detect whether unit state directory '%s' is missing, assuming it is: %m", p); + } + if (exec_directory_is_private(context, type)) { /* So, here's one extra complication when dealing with DynamicUser=1 units. In that * case we want to avoid leaving a directory around fully accessible that is owned by diff --git a/src/core/manager.c b/src/core/manager.c index 23df5ce191..8a081d0056 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -720,9 +720,9 @@ static int manager_setup_prefix(Manager *m) { static const struct table_entry paths_user[_EXEC_DIRECTORY_TYPE_MAX] = { [EXEC_DIRECTORY_RUNTIME] = { SD_PATH_USER_RUNTIME, NULL }, - [EXEC_DIRECTORY_STATE] = { SD_PATH_USER_CONFIGURATION, NULL }, + [EXEC_DIRECTORY_STATE] = { SD_PATH_USER_STATE_PRIVATE, NULL }, [EXEC_DIRECTORY_CACHE] = { SD_PATH_USER_STATE_CACHE, NULL }, - [EXEC_DIRECTORY_LOGS] = { SD_PATH_USER_CONFIGURATION, "log" }, + [EXEC_DIRECTORY_LOGS] = { SD_PATH_USER_STATE_PRIVATE, "log" }, [EXEC_DIRECTORY_CONFIGURATION] = { SD_PATH_USER_CONFIGURATION, NULL }, }; diff --git a/src/core/unit-printf.c b/src/core/unit-printf.c index 3977082cc1..9f95984eb6 100644 --- a/src/core/unit-printf.c +++ b/src/core/unit-printf.c @@ -209,8 +209,8 @@ int unit_full_printf_full(const Unit *u, const char *format, size_t max_length, * %C: the cache directory root (e.g. /var/cache or $XDG_CACHE_HOME) * %d: the credentials directory ($CREDENTIALS_DIRECTORY) * %E: the configuration directory root (e.g. /etc or $XDG_CONFIG_HOME) - * %L: the log directory root (e.g. /var/log or $XDG_CONFIG_HOME/log) - * %S: the state directory root (e.g. /var/lib or $XDG_CONFIG_HOME) + * %L: the log directory root (e.g. /var/log or $XDG_STATE_HOME/log) + * %S: the state directory root (e.g. /var/lib or $XDG_STATE_HOME) * %t: the runtime directory root (e.g. /run or $XDG_RUNTIME_DIR) * * %h: the homedir of the running user diff --git a/test/test-execute/exec-specifier-user.service b/test/test-execute/exec-specifier-user.service index ee0301a426..ab565fb4fb 100644 --- a/test/test-execute/exec-specifier-user.service +++ b/test/test-execute/exec-specifier-user.service @@ -5,7 +5,7 @@ Description=Test for specifiers [Service] Type=oneshot ExecStart=sh -c 'test %t = $$XDG_RUNTIME_DIR' -ExecStart=sh -c 'test %S = %h/.config' +ExecStart=sh -c 'test %S = %h/.local/state' ExecStart=sh -c 'test %C = %h/.cache' -ExecStart=sh -c 'test %L = %h/.config/log' +ExecStart=sh -c 'test %L = %h/.local/state/log' ExecStart=sh -c 'test %E = %h/.config' From 59dd2bbbb6fa4e5497b1cae17b76ee132f3107c1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Jun 2023 11:16:01 +0200 Subject: [PATCH 08/16] execute: associate logs from setup_exec_directory() with the unit name --- src/core/execute.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index 3e065b2ca8..f19c144f37 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2466,6 +2466,7 @@ static int create_many_symlinks(const char *root, const char *source, char **sym } static int setup_exec_directory( + Unit *u, const ExecContext *context, const ExecParameters *params, uid_t uid, @@ -2557,13 +2558,13 @@ static int setup_exec_directory( if (r < 0) goto fail; - log_notice("Unit state directory %s missing but matching configuration directory %s exists, assuming update from systemd 253 or older, creating compatibility symlink.", p, q); + log_unit_notice(u, "Unit state directory %s missing but matching configuration directory %s exists, assuming update from systemd 253 or older, creating compatibility symlink.", p, q); continue; } else if (r != -ENOENT) - log_warning_errno(r, "Unable to detect whether unit configuration directory '%s' exists, assuming not: %m", q); + log_unit_warning_errno(u, r, "Unable to detect whether unit configuration directory '%s' exists, assuming not: %m", q); } else if (r < 0) - log_warning_errno(r, "Unable to detect whether unit state directory '%s' is missing, assuming it is: %m", p); + log_unit_warning_errno(u, r, "Unable to detect whether unit state directory '%s' is missing, assuming it is: %m", p); } if (exec_directory_is_private(context, type)) { @@ -2620,9 +2621,9 @@ static int setup_exec_directory( * it over. Most likely the service has been upgraded from one that didn't use * DynamicUser=1, to one that does. */ - log_info("Found pre-existing public %s= directory %s, migrating to %s.\n" - "Apparently, service previously had DynamicUser= turned off, and has now turned it on.", - exec_directory_type_to_string(type), p, pp); + log_unit_info(u, "Found pre-existing public %s= directory %s, migrating to %s.\n" + "Apparently, service previously had DynamicUser= turned off, and has now turned it on.", + exec_directory_type_to_string(type), p, pp); if (rename(p, pp) < 0) { r = -errno; @@ -2689,9 +2690,9 @@ static int setup_exec_directory( /* Hmm, apparently DynamicUser= was once turned on for this service, * but is no longer. Let's move the directory back up. */ - log_info("Found pre-existing private %s= directory %s, migrating to %s.\n" - "Apparently, service previously had DynamicUser= turned on, and has now turned it off.", - exec_directory_type_to_string(type), q, p); + log_unit_info(u, "Found pre-existing private %s= directory %s, migrating to %s.\n" + "Apparently, service previously had DynamicUser= turned on, and has now turned it off.", + exec_directory_type_to_string(type), q, p); if (unlink(p) < 0) { r = -errno; @@ -2724,10 +2725,10 @@ static int setup_exec_directory( /* Still complain if the access mode doesn't match */ if (((st.st_mode ^ context->directories[type].mode) & 07777) != 0) - log_warning("%s \'%s\' already exists but the mode is different. " - "(File system: %o %sMode: %o)", - exec_directory_type_to_string(type), context->directories[type].items[i].path, - st.st_mode & 07777, exec_directory_type_to_string(type), context->directories[type].mode & 07777); + log_unit_warning(u, "%s \'%s\' already exists but the mode is different. " + "(File system: %o %sMode: %o)", + exec_directory_type_to_string(type), context->directories[type].items[i].path, + st.st_mode & 07777, exec_directory_type_to_string(type), context->directories[type].mode & 07777); continue; } @@ -5297,7 +5298,7 @@ static int exec_child( needs_mount_namespace = exec_needs_mount_namespace(context, params, runtime); for (ExecDirectoryType dt = 0; dt < _EXEC_DIRECTORY_TYPE_MAX; dt++) { - r = setup_exec_directory(context, params, uid, gid, dt, needs_mount_namespace, exit_status); + r = setup_exec_directory(unit, context, params, uid, gid, dt, needs_mount_namespace, exit_status); if (r < 0) return log_unit_error_errno(unit, r, "Failed to set up special execution directory in %s: %m", params->prefix[dt]); } From db58f5de3d9f0eb4897c2781fc226307b7ac0a5e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Jun 2023 11:19:12 +0200 Subject: [PATCH 09/16] execute: shorten some code by using RET_NERRNO() --- src/core/execute.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index f19c144f37..c3eeaa486b 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2625,10 +2625,9 @@ static int setup_exec_directory( "Apparently, service previously had DynamicUser= turned off, and has now turned it on.", exec_directory_type_to_string(type), p, pp); - if (rename(p, pp) < 0) { - r = -errno; + r = RET_NERRNO(rename(p, pp)); + if (r < 0) goto fail; - } } else { /* Otherwise, create the actual directory for the service */ @@ -2694,15 +2693,13 @@ static int setup_exec_directory( "Apparently, service previously had DynamicUser= turned on, and has now turned it off.", exec_directory_type_to_string(type), q, p); - if (unlink(p) < 0) { - r = -errno; + r = RET_NERRNO(unlink(p)); + if (r < 0) goto fail; - } - if (rename(q, p) < 0) { - r = -errno; + r = RET_NERRNO(rename(q, p)); + if (r < 0) goto fail; - } } } @@ -2718,10 +2715,9 @@ static int setup_exec_directory( * as in the common case it is not written to by a service, and shall * not be writable. */ - if (stat(p, &st) < 0) { - r = -errno; + r = RET_NERRNO(stat(p, &st)); + if (r < 0) goto fail; - } /* Still complain if the access mode doesn't match */ if (((st.st_mode ^ context->directories[type].mode) & 07777) != 0) From b93d24e07d903d5860f20ec97849760091348d98 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Jun 2023 11:19:31 +0200 Subject: [PATCH 10/16] execute: shorten code by making use of laccess() return code properly --- src/core/execute.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/execute.c b/src/core/execute.c index c3eeaa486b..652ff44422 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2615,7 +2615,7 @@ static int setup_exec_directory( goto fail; if (is_dir(p, false) > 0 && - (laccess(pp, F_OK) < 0 && errno == ENOENT)) { + (laccess(pp, F_OK) == -ENOENT)) { /* Hmm, the private directory doesn't exist yet, but the normal one exists? If so, move * it over. Most likely the service has been upgraded from one that didn't use From f5bb36dcfe71dab3f79e8e6133a2f4260d91f213 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Jun 2023 11:19:47 +0200 Subject: [PATCH 11/16] execute: don't bother with chowning StateDirectory= and friends in user mode --- src/core/execute.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/core/execute.c b/src/core/execute.c index 652ff44422..d850a68022 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -2738,6 +2738,11 @@ static int setup_exec_directory( if (r < 0) goto fail; + /* Skip the rest (which deals with ownership) in user mode, since ownership changes are not + * available to user code anyway */ + if (params->runtime_scope != RUNTIME_SCOPE_SYSTEM) + continue; + /* Then, change the ownership of the whole tree, if necessary. When dynamic users are used we * drop the suid/sgid bits, since we really don't want SUID/SGID files for dynamic UID/GID * assignments to exist. */ From 580a007bb6a192b5f821ace04f13694278b6618c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Jun 2023 23:23:21 +0200 Subject: [PATCH 12/16] test: add test for new XDG_STATE_HOME handling --- test/units/testsuite-23.statedir.sh | 60 +++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100755 test/units/testsuite-23.statedir.sh diff --git a/test/units/testsuite-23.statedir.sh b/test/units/testsuite-23.statedir.sh new file mode 100755 index 0000000000..b592314a09 --- /dev/null +++ b/test/units/testsuite-23.statedir.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later +# shellcheck disable=SC2235 +# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*- +# ex: ts=8 sw=4 sts=4 et filetype=sh +set -eux +set -o pipefail + +# Test unit configuration/state/cache/log/runtime data cleanup + +export HOME=/root +export XDG_RUNTIME_DIR=/run/user/0 + +systemctl start user@0.service + +( ! test -d "$HOME"/.local/state/foo) +( ! test -d "$HOME"/.config/foo) + +systemd-run --user -p StateDirectory=foo --wait /bin/true + +test -d "$HOME"/.local/state/foo +( ! test -L "$HOME"/.local/state/foo) +( ! test -d "$HOME"/.config/foo) + +systemd-run --user -p StateDirectory=foo -p ConfigurationDirectory=foo --wait /bin/true + +test -d "$HOME"/.local/state/foo +( ! test -L "$HOME"/.local/state/foo) +test -d "$HOME"/.config/foo + +rmdir "$HOME"/.local/state/foo "$HOME"/.config/foo + +systemd-run --user -p StateDirectory=foo -p ConfigurationDirectory=foo --wait /bin/true + +test -d "$HOME"/.local/state/foo +( ! test -L "$HOME"/.local/state/foo) +test -d "$HOME"/.config/foo + +rmdir "$HOME"/.local/state/foo "$HOME"/.config/foo + +# Now trigger an update scenario by creating a config dir first +systemd-run --user -p ConfigurationDirectory=foo --wait /bin/true + +( ! test -d "$HOME"/.local/state/foo) +test -d "$HOME"/.config/foo + +# This will look like an update and result in a symlink +systemd-run --user -p StateDirectory=foo -p ConfigurationDirectory=foo --wait /bin/true + +test -d "$HOME"/.local/state/foo +test -L "$HOME"/.local/state/foo +test -d "$HOME"/.config/foo + +test "$(readlink "$HOME"/.local/state/foo)" = ../../.config/foo + +# Check that this will work safely a second time +systemd-run --user -p StateDirectory=foo -p ConfigurationDirectory=foo --wait /bin/true + +rm "$HOME"/.local/state/foo +rmdir "$HOME"/.config/foo From b4d6bc63e602048188896110a585aa7de1c70c9b Mon Sep 17 00:00:00 2001 From: Franklin Yu Date: Thu, 25 May 2023 22:06:54 -0700 Subject: [PATCH 13/16] man: mention the newly-added XDG_STATE_HOME The description is copied from config-home. Taken from: #27795 --- man/file-hierarchy.xml | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/man/file-hierarchy.xml b/man/file-hierarchy.xml index 9e91af9060..faf549a2f3 100644 --- a/man/file-hierarchy.xml +++ b/man/file-hierarchy.xml @@ -518,13 +518,10 @@ ~/.config/ - Application configuration and state. When a - new user is created, this directory will be empty or not exist - at all. Applications should fall back to defaults should their - configuration or state in this directory be missing. If an - application finds $XDG_CONFIG_HOME set, it - should use the directory specified in it instead of this - directory. + Application configuration. When a new user is created, this directory will be empty + or not exist at all. Applications should fall back to defaults should their configuration in this + directory be missing. If an application finds $XDG_CONFIG_HOME set, it should use + the directory specified in it instead of this directory. @@ -570,6 +567,15 @@ directory. + + ~/.local/state/ + + Application state. When a new user is created, this directory will be empty or not + exist at all. Applications should fall back to defaults should their state in this directory be + missing. If an application finds $XDG_STATE_HOME set, it should use the directory + specified in it instead of this directory. + + From fa1d34825a9b410275e716b9b70f4fca02c71ba9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Jun 2023 10:28:44 +0200 Subject: [PATCH 14/16] man: rebreak lines in file-hierarchy(7) a bit (Does not change a single word, just rebreaks a bunch of paragraphs matching our current line breaking rules) --- man/file-hierarchy.xml | 47 ++++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/man/file-hierarchy.xml b/man/file-hierarchy.xml index faf549a2f3..5c9b76674c 100644 --- a/man/file-hierarchy.xml +++ b/man/file-hierarchy.xml @@ -505,13 +505,10 @@ ~/.cache/ - Persistent user cache data. User programs may - place non-essential data in this directory. Flushing this - directory should have no effect on operation of programs, - except for increased runtimes necessary to rebuild these - caches. If an application finds - $XDG_CACHE_HOME set, it should use the - directory specified in it instead of this + Persistent user cache data. User programs may place non-essential data in this + directory. Flushing this directory should have no effect on operation of programs, except for + increased runtimes necessary to rebuild these caches. If an application finds + $XDG_CACHE_HOME set, it should use the directory specified in it instead of this directory. @@ -527,44 +524,36 @@ ~/.local/bin/ - Executables that shall appear in the user's - $PATH search path. It is recommended not to - place executables in this directory that are not useful for - invocation from a shell; these should be placed in a - subdirectory of ~/.local/lib/ instead. - Care should be taken when placing architecture-dependent - binaries in this place, which might be problematic if the home - directory is shared between multiple hosts with different + Executables that shall appear in the user's $PATH search path. It + is recommended not to place executables in this directory that are not useful for invocation from a + shell; these should be placed in a subdirectory of ~/.local/lib/ instead. Care + should be taken when placing architecture-dependent binaries in this place, which might be + problematic if the home directory is shared between multiple hosts with different architectures. ~/.local/lib/ - Static, private vendor data that is compatible - with all architectures. + Static, private vendor data that is compatible with all + architectures. ~/.local/lib/arch-id/ - Location for placing public dynamic libraries. - The architecture identifier to use is defined on Multiarch - Architecture Specifiers (Tuples) - list. + Location for placing public dynamic libraries. The architecture identifier to use is + defined on Multiarch Architecture Specifiers + (Tuples) list. ~/.local/share/ - Resources shared between multiple packages, - such as fonts or artwork. Usually, the precise location and - format of files stored below this directory is subject to - specifications that ensure interoperability. If an application - finds $XDG_DATA_HOME set, it should use the - directory specified in it instead of this - directory. + Resources shared between multiple packages, such as fonts or artwork. Usually, the + precise location and format of files stored below this directory is subject to specifications that + ensure interoperability. If an application finds $XDG_DATA_HOME set, it should use + the directory specified in it instead of this directory. From cc8fdd5d307a620700d4729d74143ca434f0707c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Jun 2023 10:32:11 +0200 Subject: [PATCH 15/16] man: properly close XML tags --- man/systemd.exec.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index 32128d4fab..3960deac3a 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -1447,7 +1447,7 @@ CapabilityBoundingSet=~CAP_B CAP_C The second parameter will be interpreted as a destination path that will be created as a symlink to the directory. The symlinks will be created after any BindPaths= or TemporaryFileSystem= options have been set up, to make ephemeral symlinking possible. The same source can have multiple symlinks, by - using the same first parameter, but a different second parameter. + using the same first parameter, but a different second parameter. The directories defined by these options are always created under the standard paths used by systemd (/var/, /run/, /etc/, …). If the service needs @@ -1483,7 +1483,7 @@ StateDirectory=aaa/bbb ccc RuntimeDirectory=foo:bar foo:baz the service manager creates /run/foo (if it does not exist), and /run/bar plus /run/baz as symlinks to - /run/foo. + /run/foo. From b50aadaff22f9b3ad3bbcbfd2edd661456a5b4bf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Jun 2023 13:14:17 +0200 Subject: [PATCH 16/16] tmpfiles: teach tmpfiles the new XDG_STATE_HOME variable too --- man/tmpfiles.d.xml | 4 ++-- src/tmpfiles/tmpfiles.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml index 495315d55c..4c972aa985 100644 --- a/man/tmpfiles.d.xml +++ b/man/tmpfiles.d.xml @@ -736,7 +736,7 @@ d /tmp/foo/bar - - - bmA:1h - %L System or user log directory - In mode, this is the same as $XDG_CONFIG_HOME with /log appended, and /var/log otherwise. + In mode, this is the same as $XDG_STATE_HOME with /log appended, and /var/log otherwise. @@ -744,7 +744,7 @@ d /tmp/foo/bar - - - bmA:1h - %S System or user state directory - In mode, this is the same as $XDG_CONFIG_HOME, and /var/lib otherwise. + In mode, this is the same as $XDG_STATE_HOME, and /var/lib otherwise. %t diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index eabac56320..a7de3c87fe 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -255,9 +255,9 @@ static int specifier_directory(char specifier, const void *data, const char *roo static const struct table_entry paths_user[] = { [DIRECTORY_RUNTIME] = { SD_PATH_USER_RUNTIME }, - [DIRECTORY_STATE] = { SD_PATH_USER_CONFIGURATION }, + [DIRECTORY_STATE] = { SD_PATH_USER_STATE_PRIVATE }, [DIRECTORY_CACHE] = { SD_PATH_USER_STATE_CACHE }, - [DIRECTORY_LOGS] = { SD_PATH_USER_CONFIGURATION, "log" }, + [DIRECTORY_LOGS] = { SD_PATH_USER_STATE_PRIVATE, "log" }, }; const struct table_entry *paths;