From d4d99bc6e4d90da335c62c7a75a2a12e44b428a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Aug 2019 12:48:41 +0200 Subject: [PATCH 01/10] basic/cgroup-util: let cgroup_unified_flush() return the detected hierarchy This avoid the use of the global variable. Also rename cgroup_unified_update() to cgroup_unified_cached() and cgroup_unified_flush() to cgroup_unified() to better reflect their new roles. --- src/basic/cgroup-util.c | 61 ++++++++++++-------------- src/basic/cgroup-util.h | 5 ++- src/core/cgroup.c | 2 +- src/libsystemd/sd-bus/test-bus-creds.c | 2 +- src/nspawn/nspawn.c | 2 +- src/test/test-cgroup-util.c | 4 +- src/test/test-condition.c | 2 +- 7 files changed, 38 insertions(+), 40 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 7b5839ccd6..2865cd518e 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -2526,20 +2526,20 @@ int cg_kernel_controllers(Set **ret) { return 0; } -static thread_local CGroupUnified unified_cache = CGROUP_UNIFIED_UNKNOWN; - -/* The hybrid mode was initially implemented in v232 and simply mounted cgroup2 on /sys/fs/cgroup/systemd. This - * unfortunately broke other tools (such as docker) which expected the v1 "name=systemd" hierarchy on - * /sys/fs/cgroup/systemd. From v233 and on, the hybrid mode mountnbs v2 on /sys/fs/cgroup/unified and maintains - * "name=systemd" hierarchy on /sys/fs/cgroup/systemd for compatibility with other tools. +/* The hybrid mode was initially implemented in v232 and simply mounted cgroup2 on + * /sys/fs/cgroup/systemd. This unfortunately broke other tools (such as docker) which expected the v1 + * "name=systemd" hierarchy on /sys/fs/cgroup/systemd. From v233 and on, the hybrid mode mounts v2 on + * /sys/fs/cgroup/unified and maintains "name=systemd" hierarchy on /sys/fs/cgroup/systemd for compatibility + * with other tools. * - * To keep live upgrade working, we detect and support v232 layout. When v232 layout is detected, to keep cgroup v2 - * process management but disable the compat dual layout, we return %true on - * cg_unified_controller(SYSTEMD_CGROUP_CONTROLLER) and %false on cg_hybrid_unified(). + * To keep live upgrade working, we detect and support v232 layout. When v232 layout is detected, to keep + * cgroup v2 process management but disable the compat dual layout, we return true on + * cg_unified_controller(SYSTEMD_CGROUP_CONTROLLER) and false on cg_hybrid_unified(). */ static thread_local bool unified_systemd_v232; -static int cg_unified_update(void) { +int cg_unified_cached(bool flush) { + static thread_local CGroupUnified unified_cache = CGROUP_UNIFIED_UNKNOWN; struct statfs fs; @@ -2548,8 +2548,10 @@ static int cg_unified_update(void) { * have any other trouble determining if the unified hierarchy * is supported. */ - if (unified_cache >= CGROUP_UNIFIED_NONE) - return 0; + if (flush) + unified_cache = CGROUP_UNIFIED_UNKNOWN; + else if (unified_cache >= CGROUP_UNIFIED_NONE) + return unified_cache; if (statfs("/sys/fs/cgroup/", &fs) < 0) return log_debug_errno(errno, "statfs(\"/sys/fs/cgroup/\") failed: %m"); @@ -2585,20 +2587,20 @@ static int cg_unified_update(void) { "Unknown filesystem type %llx mounted on /sys/fs/cgroup.", (unsigned long long)fs.f_type); - return 0; + return unified_cache; } int cg_unified_controller(const char *controller) { int r; - r = cg_unified_update(); + r = cg_unified_cached(false); if (r < 0) return r; - if (unified_cache == CGROUP_UNIFIED_NONE) + if (r == CGROUP_UNIFIED_NONE) return false; - if (unified_cache >= CGROUP_UNIFIED_ALL) + if (r >= CGROUP_UNIFIED_ALL) return true; return streq_ptr(controller, SYSTEMD_CGROUP_CONTROLLER); @@ -2607,27 +2609,21 @@ int cg_unified_controller(const char *controller) { int cg_all_unified(void) { int r; - r = cg_unified_update(); + r = cg_unified_cached(false); if (r < 0) return r; - return unified_cache >= CGROUP_UNIFIED_ALL; + return r >= CGROUP_UNIFIED_ALL; } int cg_hybrid_unified(void) { int r; - r = cg_unified_update(); + r = cg_unified_cached(false); if (r < 0) return r; - return unified_cache == CGROUP_UNIFIED_SYSTEMD && !unified_systemd_v232; -} - -int cg_unified_flush(void) { - unified_cache = CGROUP_UNIFIED_UNKNOWN; - - return cg_unified_update(); + return r == CGROUP_UNIFIED_SYSTEMD && !unified_systemd_v232; } int cg_enable_everywhere( @@ -2740,10 +2736,10 @@ int cg_enable_everywhere( bool cg_is_unified_wanted(void) { static thread_local int wanted = -1; - int r; bool b; const bool is_default = DEFAULT_HIERARCHY == CGROUP_UNIFIED_ALL; _cleanup_free_ char *c = NULL; + int r; /* If we have a cached value, return that. */ if (wanted >= 0) @@ -2751,8 +2747,9 @@ bool cg_is_unified_wanted(void) { /* If the hierarchy is already mounted, then follow whatever * was chosen for it. */ - if (cg_unified_flush() >= 0) - return (wanted = unified_cache >= CGROUP_UNIFIED_ALL); + r = cg_unified_cached(true); + if (r >= 0) + return (wanted = r >= CGROUP_UNIFIED_ALL); /* If we were explicitly passed systemd.unified_cgroup_hierarchy, * respect that. */ @@ -2777,8 +2774,7 @@ bool cg_is_legacy_wanted(void) { return wanted; /* Check if we have cgroup v2 already mounted. */ - if (cg_unified_flush() >= 0 && - unified_cache == CGROUP_UNIFIED_ALL) + if (cg_unified_cached(true) == CGROUP_UNIFIED_ALL) return (wanted = false); /* Otherwise, assume that at least partial legacy is wanted, @@ -2801,8 +2797,7 @@ bool cg_is_hybrid_wanted(void) { /* If the hierarchy is already mounted, then follow whatever * was chosen for it. */ - if (cg_unified_flush() >= 0 && - unified_cache == CGROUP_UNIFIED_ALL) + if (cg_unified_cached(true) == CGROUP_UNIFIED_ALL) return (wanted = false); /* Otherwise, let's see what the kernel command line has to say. diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index a39ab451b9..ba8df8139d 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -258,7 +258,10 @@ bool cg_ns_supported(void); int cg_all_unified(void); int cg_hybrid_unified(void); int cg_unified_controller(const char *controller); -int cg_unified_flush(void); +int cg_unified_cached(bool flush); +static inline int cg_unified(void) { + return cg_unified_cached(true); +} bool cg_is_unified_wanted(void); bool cg_is_legacy_wanted(void); diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 60a7799361..7b0a41fbc8 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -2839,7 +2839,7 @@ int manager_setup_cgroup(Manager *m) { if (r < 0) return log_error_errno(r, "Cannot find cgroup mount point: %m"); - r = cg_unified_flush(); + r = cg_unified(); if (r < 0) return log_error_errno(r, "Couldn't determine if we are running in the unified hierarchy: %m"); diff --git a/src/libsystemd/sd-bus/test-bus-creds.c b/src/libsystemd/sd-bus/test-bus-creds.c index c02c459663..7f7bc491d2 100644 --- a/src/libsystemd/sd-bus/test-bus-creds.c +++ b/src/libsystemd/sd-bus/test-bus-creds.c @@ -13,7 +13,7 @@ int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); - if (cg_unified_flush() == -ENOMEDIUM) + if (cg_unified() == -ENOMEDIUM) return log_tests_skipped("/sys/fs/cgroup/ not available"); r = sd_bus_creds_new_from_pid(&creds, 0, _SD_BUS_CREDS_ALL); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 2aec8041f0..91ec61402b 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -4720,7 +4720,7 @@ static int run(int argc, char *argv[]) { if (r < 0) goto finish; - r = cg_unified_flush(); + r = cg_unified(); if (r < 0) { log_error_errno(r, "Failed to determine whether the unified cgroups hierarchy is used: %m"); goto finish; diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c index b54b5e76c6..f45c5ff760 100644 --- a/src/test/test-cgroup-util.c +++ b/src/test/test-cgroup-util.c @@ -385,12 +385,12 @@ static void test_is_wanted(void) { static void test_cg_tests(void) { int all, hybrid, systemd, r; - r = cg_unified_flush(); + r = cg_unified(); if (r == -ENOMEDIUM) { log_notice_errno(r, "Skipping cg hierarchy tests: %m"); return; } - assert_se(r == 0); + assert_se(r >= 0); all = cg_all_unified(); assert_se(IN_SET(all, 0, 1)); diff --git a/src/test/test-condition.c b/src/test/test-condition.c index a79263a50b..9a0a4cfee2 100644 --- a/src/test/test-condition.c +++ b/src/test/test-condition.c @@ -124,7 +124,7 @@ static void test_condition_test_control_group_controller(void) { _cleanup_free_ char *controller_name = NULL; int r; - r = cg_unified_flush(); + r = cg_unified(); if (r < 0) { log_notice_errno(r, "Skipping ConditionControlGroupController tests: %m"); return; From 55fced5a1983cbe03197d782acdd673c035ba0ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Aug 2019 13:10:49 +0200 Subject: [PATCH 02/10] util-lib: move yes_no() and friends to string-util.h --- src/basic/string-util.h | 16 ++++++++++++++++ src/basic/util.h | 16 ---------------- src/nspawn/test-patch-uid.c | 2 +- src/shared/serialize.h | 1 + src/test/test-capability.c | 2 +- 5 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/basic/string-util.h b/src/basic/string-util.h index 76767afcac..3981129db3 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -45,6 +45,22 @@ static inline const char *strna(const char *s) { return s ?: "n/a"; } +static inline const char* yes_no(bool b) { + return b ? "yes" : "no"; +} + +static inline const char* true_false(bool b) { + return b ? "true" : "false"; +} + +static inline const char* one_zero(bool b) { + return b ? "1" : "0"; +} + +static inline const char* enable_disable(bool b) { + return b ? "enable" : "disable"; +} + static inline bool isempty(const char *p) { return !p || !p[0]; } diff --git a/src/basic/util.h b/src/basic/util.h index 25e6ab8112..6fc7480fcb 100644 --- a/src/basic/util.h +++ b/src/basic/util.h @@ -5,22 +5,6 @@ #include "macro.h" -static inline const char* yes_no(bool b) { - return b ? "yes" : "no"; -} - -static inline const char* true_false(bool b) { - return b ? "true" : "false"; -} - -static inline const char* one_zero(bool b) { - return b ? "1" : "0"; -} - -static inline const char* enable_disable(bool b) { - return b ? "enable" : "disable"; -} - extern int saved_argc; extern char **saved_argv; diff --git a/src/nspawn/test-patch-uid.c b/src/nspawn/test-patch-uid.c index b50f0990d8..a6829629b4 100644 --- a/src/nspawn/test-patch-uid.c +++ b/src/nspawn/test-patch-uid.c @@ -5,8 +5,8 @@ #include "log.h" #include "nspawn-patch-uid.h" #include "user-util.h" +#include "string-util.h" #include "tests.h" -#include "util.h" int main(int argc, char *argv[]) { uid_t shift, range; diff --git a/src/shared/serialize.h b/src/shared/serialize.h index 8f4efc7996..9e61c825aa 100644 --- a/src/shared/serialize.h +++ b/src/shared/serialize.h @@ -5,6 +5,7 @@ #include "fdset.h" #include "macro.h" +#include "string-util.h" #include "time-util.h" int serialize_item(FILE *f, const char *key, const char *value); diff --git a/src/test/test-capability.c b/src/test/test-capability.c index f9fae84dde..84a6e92df1 100644 --- a/src/test/test-capability.c +++ b/src/test/test-capability.c @@ -15,8 +15,8 @@ #include "macro.h" #include "missing_prctl.h" #include "parse-util.h" +#include "string-util.h" #include "tests.h" -#include "util.h" static uid_t test_uid = -1; static gid_t test_gid = -1; From fdb3decaa7ca791906f87dafc9d641c4cee4ec26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Aug 2019 13:14:45 +0200 Subject: [PATCH 03/10] util-lib: move some functions from basic/cgroup-util to shared/cgroup-setup This way less stuff needs to be in basic. Initially, I wanted to move all the parts of cgroup-utils.[ch] that depend on efivars.[ch] to shared, because efivars.[ch] is in shared/. Later on, I decide to split efivars.[ch], so the move done in this patch is not necessary anymore. Nevertheless, it is still valid on its own. If at some point we want to expose libbasic, it is better to to not have stuff that belong in libshared there. --- src/basic/cgroup-util.c | 846 ---------------------------------- src/basic/cgroup-util.h | 26 -- src/core/cgroup.c | 1 + src/core/execute.c | 1 + src/core/load-fragment.c | 2 +- src/core/mount-setup.c | 1 + src/core/unit.c | 1 + src/login/pam_systemd.c | 2 +- src/nspawn/nspawn-cgroup.c | 1 + src/shared/bus-unit-util.c | 1 + src/shared/cgroup-setup.c | 860 +++++++++++++++++++++++++++++++++++ src/shared/cgroup-setup.h | 34 ++ src/shared/meson.build | 2 + src/shutdown/shutdown.c | 1 + src/test/meson.build | 4 + src/test/test-cgroup-setup.c | 67 +++ src/test/test-cgroup-util.c | 52 --- src/test/test-cgroup.c | 1 + src/test/test-helper.c | 2 +- 19 files changed, 978 insertions(+), 927 deletions(-) create mode 100644 src/shared/cgroup-setup.c create mode 100644 src/shared/cgroup-setup.h create mode 100644 src/test/test-cgroup-setup.c diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 2865cd518e..6582b92d67 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -31,7 +31,6 @@ #include "mkdir.h" #include "parse-util.h" #include "path-util.h" -#include "proc-cmdline.h" #include "process-util.h" #include "set.h" #include "special.h" @@ -410,173 +409,6 @@ int cg_kill_recursive( return ret; } -int cg_migrate( - const char *cfrom, - const char *pfrom, - const char *cto, - const char *pto, - CGroupFlags flags) { - - bool done = false; - _cleanup_set_free_ Set *s = NULL; - int r, ret = 0; - pid_t my_pid; - - assert(cfrom); - assert(pfrom); - assert(cto); - assert(pto); - - s = set_new(NULL); - if (!s) - return -ENOMEM; - - my_pid = getpid_cached(); - - do { - _cleanup_fclose_ FILE *f = NULL; - pid_t pid = 0; - done = true; - - r = cg_enumerate_processes(cfrom, pfrom, &f); - if (r < 0) { - if (ret >= 0 && r != -ENOENT) - return r; - - return ret; - } - - while ((r = cg_read_pid(f, &pid)) > 0) { - - /* This might do weird stuff if we aren't a - * single-threaded program. However, we - * luckily know we are not */ - if ((flags & CGROUP_IGNORE_SELF) && pid == my_pid) - continue; - - if (set_get(s, PID_TO_PTR(pid)) == PID_TO_PTR(pid)) - continue; - - /* Ignore kernel threads. Since they can only - * exist in the root cgroup, we only check for - * them there. */ - if (cfrom && - empty_or_root(pfrom) && - is_kernel_thread(pid) > 0) - continue; - - r = cg_attach(cto, pto, pid); - if (r < 0) { - if (ret >= 0 && r != -ESRCH) - ret = r; - } else if (ret == 0) - ret = 1; - - done = false; - - r = set_put(s, PID_TO_PTR(pid)); - if (r < 0) { - if (ret >= 0) - return r; - - return ret; - } - } - - if (r < 0) { - if (ret >= 0) - return r; - - return ret; - } - } while (!done); - - return ret; -} - -int cg_migrate_recursive( - const char *cfrom, - const char *pfrom, - const char *cto, - const char *pto, - CGroupFlags flags) { - - _cleanup_closedir_ DIR *d = NULL; - int r, ret = 0; - char *fn; - - assert(cfrom); - assert(pfrom); - assert(cto); - assert(pto); - - ret = cg_migrate(cfrom, pfrom, cto, pto, flags); - - r = cg_enumerate_subgroups(cfrom, pfrom, &d); - if (r < 0) { - if (ret >= 0 && r != -ENOENT) - return r; - - return ret; - } - - while ((r = cg_read_subgroup(d, &fn)) > 0) { - _cleanup_free_ char *p = NULL; - - p = path_join(empty_to_root(pfrom), fn); - free(fn); - if (!p) - return -ENOMEM; - - r = cg_migrate_recursive(cfrom, p, cto, pto, flags); - if (r != 0 && ret >= 0) - ret = r; - } - - if (r < 0 && ret >= 0) - ret = r; - - if (flags & CGROUP_REMOVE) { - r = cg_rmdir(cfrom, pfrom); - if (r < 0 && ret >= 0 && !IN_SET(r, -ENOENT, -EBUSY)) - return r; - } - - return ret; -} - -int cg_migrate_recursive_fallback( - const char *cfrom, - const char *pfrom, - const char *cto, - const char *pto, - CGroupFlags flags) { - - int r; - - assert(cfrom); - assert(pfrom); - assert(cto); - assert(pto); - - r = cg_migrate_recursive(cfrom, pfrom, cto, pto, flags); - if (r < 0) { - char prefix[strlen(pto) + 1]; - - /* This didn't work? Then let's try all prefixes of the destination */ - - PATH_FOREACH_PREFIX(prefix, pto) { - int q; - - q = cg_migrate_recursive(cfrom, pfrom, cto, prefix, flags); - if (q >= 0) - return q; - } - } - - return r; -} - static const char *controller_to_dirname(const char *controller) { const char *e; @@ -742,253 +574,6 @@ int cg_get_path_and_check(const char *controller, const char *path, const char * return cg_get_path(controller, path, suffix, fs); } -static int trim_cb(const char *path, const struct stat *sb, int typeflag, struct FTW *ftwbuf) { - assert(path); - assert(sb); - assert(ftwbuf); - - if (typeflag != FTW_DP) - return 0; - - if (ftwbuf->level < 1) - return 0; - - (void) rmdir(path); - return 0; -} - -int cg_trim(const char *controller, const char *path, bool delete_root) { - _cleanup_free_ char *fs = NULL; - int r = 0, q; - - assert(path); - - r = cg_get_path(controller, path, NULL, &fs); - if (r < 0) - return r; - - errno = 0; - if (nftw(fs, trim_cb, 64, FTW_DEPTH|FTW_MOUNT|FTW_PHYS) != 0) { - if (errno == ENOENT) - r = 0; - else - r = errno_or_else(EIO); - } - - if (delete_root) { - if (rmdir(fs) < 0 && errno != ENOENT) - return -errno; - } - - q = cg_hybrid_unified(); - if (q < 0) - return q; - if (q > 0 && streq(controller, SYSTEMD_CGROUP_CONTROLLER)) { - q = cg_trim(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, delete_root); - if (q < 0) - log_warning_errno(q, "Failed to trim compat systemd cgroup %s: %m", path); - } - - return r; -} - -/* Create a cgroup in the hierarchy of controller. - * Returns 0 if the group already existed, 1 on success, negative otherwise. - */ -int cg_create(const char *controller, const char *path) { - _cleanup_free_ char *fs = NULL; - int r; - - r = cg_get_path_and_check(controller, path, NULL, &fs); - if (r < 0) - return r; - - r = mkdir_parents(fs, 0755); - if (r < 0) - return r; - - r = mkdir_errno_wrapper(fs, 0755); - if (r == -EEXIST) - return 0; - if (r < 0) - return r; - - r = cg_hybrid_unified(); - if (r < 0) - return r; - - if (r > 0 && streq(controller, SYSTEMD_CGROUP_CONTROLLER)) { - r = cg_create(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path); - if (r < 0) - log_warning_errno(r, "Failed to create compat systemd cgroup %s: %m", path); - } - - return 1; -} - -int cg_create_and_attach(const char *controller, const char *path, pid_t pid) { - int r, q; - - assert(pid >= 0); - - r = cg_create(controller, path); - if (r < 0) - return r; - - q = cg_attach(controller, path, pid); - if (q < 0) - return q; - - /* This does not remove the cgroup on failure */ - return r; -} - -int cg_attach(const char *controller, const char *path, pid_t pid) { - _cleanup_free_ char *fs = NULL; - char c[DECIMAL_STR_MAX(pid_t) + 2]; - int r; - - assert(path); - assert(pid >= 0); - - r = cg_get_path_and_check(controller, path, "cgroup.procs", &fs); - if (r < 0) - return r; - - if (pid == 0) - pid = getpid_cached(); - - xsprintf(c, PID_FMT "\n", pid); - - r = write_string_file(fs, c, WRITE_STRING_FILE_DISABLE_BUFFER); - if (r < 0) - return r; - - r = cg_hybrid_unified(); - if (r < 0) - return r; - - if (r > 0 && streq(controller, SYSTEMD_CGROUP_CONTROLLER)) { - r = cg_attach(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, pid); - if (r < 0) - log_warning_errno(r, "Failed to attach "PID_FMT" to compat systemd cgroup %s: %m", pid, path); - } - - return 0; -} - -int cg_attach_fallback(const char *controller, const char *path, pid_t pid) { - int r; - - assert(controller); - assert(path); - assert(pid >= 0); - - r = cg_attach(controller, path, pid); - if (r < 0) { - char prefix[strlen(path) + 1]; - - /* This didn't work? Then let's try all prefixes of - * the destination */ - - PATH_FOREACH_PREFIX(prefix, path) { - int q; - - q = cg_attach(controller, prefix, pid); - if (q >= 0) - return q; - } - } - - return r; -} - -int cg_set_access( - const char *controller, - const char *path, - uid_t uid, - gid_t gid) { - - struct Attribute { - const char *name; - bool fatal; - }; - - /* cgroup v1, aka legacy/non-unified */ - static const struct Attribute legacy_attributes[] = { - { "cgroup.procs", true }, - { "tasks", false }, - { "cgroup.clone_children", false }, - {}, - }; - - /* cgroup v2, aka unified */ - static const struct Attribute unified_attributes[] = { - { "cgroup.procs", true }, - { "cgroup.subtree_control", true }, - { "cgroup.threads", false }, - {}, - }; - - static const struct Attribute* const attributes[] = { - [false] = legacy_attributes, - [true] = unified_attributes, - }; - - _cleanup_free_ char *fs = NULL; - const struct Attribute *i; - int r, unified; - - assert(path); - - if (uid == UID_INVALID && gid == GID_INVALID) - return 0; - - unified = cg_unified_controller(controller); - if (unified < 0) - return unified; - - /* Configure access to the cgroup itself */ - r = cg_get_path(controller, path, NULL, &fs); - if (r < 0) - return r; - - r = chmod_and_chown(fs, 0755, uid, gid); - if (r < 0) - return r; - - /* Configure access to the cgroup's attributes */ - for (i = attributes[unified]; i->name; i++) { - fs = mfree(fs); - - r = cg_get_path(controller, path, i->name, &fs); - if (r < 0) - return r; - - r = chmod_and_chown(fs, 0644, uid, gid); - if (r < 0) { - if (i->fatal) - return r; - - log_debug_errno(r, "Failed to set access on cgroup %s, ignoring: %m", fs); - } - } - - if (streq(controller, SYSTEMD_CGROUP_CONTROLLER)) { - r = cg_hybrid_unified(); - if (r < 0) - return r; - if (r > 0) { - /* Always propagate access mode from unified to legacy controller */ - r = cg_set_access(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, uid, gid); - if (r < 0) - log_debug_errno(r, "Failed to set access on compatibility systemd cgroup %s, ignoring: %m", path); - } - } - - return 0; -} - int cg_set_xattr(const char *controller, const char *path, const char *name, const void *value, size_t size, int flags) { _cleanup_free_ char *fs = NULL; int r; @@ -2143,194 +1728,6 @@ fail: done: memcpy(ret_values, v, sizeof(char*) * n); return 0; - -} - -int cg_create_everywhere(CGroupMask supported, CGroupMask mask, const char *path) { - CGroupController c; - CGroupMask done; - bool created; - int r; - - /* This one will create a cgroup in our private tree, but also - * duplicate it in the trees specified in mask, and remove it - * in all others. - * - * Returns 0 if the group already existed in the systemd hierarchy, - * 1 on success, negative otherwise. - */ - - /* First create the cgroup in our own hierarchy. */ - r = cg_create(SYSTEMD_CGROUP_CONTROLLER, path); - if (r < 0) - return r; - created = r; - - /* If we are in the unified hierarchy, we are done now */ - r = cg_all_unified(); - if (r < 0) - return r; - if (r > 0) - return created; - - supported &= CGROUP_MASK_V1; - mask = CGROUP_MASK_EXTEND_JOINED(mask); - done = 0; - - /* Otherwise, do the same in the other hierarchies */ - for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { - CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); - const char *n; - - if (!FLAGS_SET(supported, bit)) - continue; - - if (FLAGS_SET(done, bit)) - continue; - - n = cgroup_controller_to_string(c); - if (FLAGS_SET(mask, bit)) - (void) cg_create(n, path); - else - (void) cg_trim(n, path, true); - - done |= CGROUP_MASK_EXTEND_JOINED(bit); - } - - return created; -} - -int cg_attach_everywhere(CGroupMask supported, const char *path, pid_t pid, cg_migrate_callback_t path_callback, void *userdata) { - CGroupController c; - CGroupMask done; - int r; - - r = cg_attach(SYSTEMD_CGROUP_CONTROLLER, path, pid); - if (r < 0) - return r; - - r = cg_all_unified(); - if (r < 0) - return r; - if (r > 0) - return 0; - - supported &= CGROUP_MASK_V1; - done = 0; - - for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { - CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); - const char *p = NULL; - - if (!FLAGS_SET(supported, bit)) - continue; - - if (FLAGS_SET(done, bit)) - continue; - - if (path_callback) - p = path_callback(bit, userdata); - if (!p) - p = path; - - (void) cg_attach_fallback(cgroup_controller_to_string(c), p, pid); - done |= CGROUP_MASK_EXTEND_JOINED(bit); - } - - return 0; -} - -int cg_attach_many_everywhere(CGroupMask supported, const char *path, Set* pids, cg_migrate_callback_t path_callback, void *userdata) { - Iterator i; - void *pidp; - int r = 0; - - SET_FOREACH(pidp, pids, i) { - pid_t pid = PTR_TO_PID(pidp); - int q; - - q = cg_attach_everywhere(supported, path, pid, path_callback, userdata); - if (q < 0 && r >= 0) - r = q; - } - - return r; -} - -int cg_migrate_everywhere(CGroupMask supported, const char *from, const char *to, cg_migrate_callback_t to_callback, void *userdata) { - CGroupController c; - CGroupMask done; - int r = 0, q; - - if (!path_equal(from, to)) { - r = cg_migrate_recursive(SYSTEMD_CGROUP_CONTROLLER, from, SYSTEMD_CGROUP_CONTROLLER, to, CGROUP_REMOVE); - if (r < 0) - return r; - } - - q = cg_all_unified(); - if (q < 0) - return q; - if (q > 0) - return r; - - supported &= CGROUP_MASK_V1; - done = 0; - - for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { - CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); - const char *p = NULL; - - if (!FLAGS_SET(supported, bit)) - continue; - - if (FLAGS_SET(done, bit)) - continue; - - if (to_callback) - p = to_callback(bit, userdata); - if (!p) - p = to; - - (void) cg_migrate_recursive_fallback(SYSTEMD_CGROUP_CONTROLLER, to, cgroup_controller_to_string(c), p, 0); - done |= CGROUP_MASK_EXTEND_JOINED(bit); - } - - return r; -} - -int cg_trim_everywhere(CGroupMask supported, const char *path, bool delete_root) { - CGroupController c; - CGroupMask done; - int r, q; - - r = cg_trim(SYSTEMD_CGROUP_CONTROLLER, path, delete_root); - if (r < 0) - return r; - - q = cg_all_unified(); - if (q < 0) - return q; - if (q > 0) - return r; - - supported &= CGROUP_MASK_V1; - done = 0; - - for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { - CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); - - if (!FLAGS_SET(supported, bit)) - continue; - - if (FLAGS_SET(done, bit)) - continue; - - (void) cg_trim(cgroup_controller_to_string(c), path, delete_root); - done |= CGROUP_MASK_EXTEND_JOINED(bit); - } - - return r; } int cg_mask_to_string(CGroupMask mask, char **ret) { @@ -2626,209 +2023,6 @@ int cg_hybrid_unified(void) { return r == CGROUP_UNIFIED_SYSTEMD && !unified_systemd_v232; } -int cg_enable_everywhere( - CGroupMask supported, - CGroupMask mask, - const char *p, - CGroupMask *ret_result_mask) { - - _cleanup_fclose_ FILE *f = NULL; - _cleanup_free_ char *fs = NULL; - CGroupController c; - CGroupMask ret = 0; - int r; - - assert(p); - - if (supported == 0) { - if (ret_result_mask) - *ret_result_mask = 0; - return 0; - } - - r = cg_all_unified(); - if (r < 0) - return r; - if (r == 0) { - /* On the legacy hierarchy there's no concept of "enabling" controllers in cgroups defined. Let's claim - * complete success right away. (If you wonder why we return the full mask here, rather than zero: the - * caller tends to use the returned mask later on to compare if all controllers where properly joined, - * and if not requeues realization. This use is the primary purpose of the return value, hence let's - * minimize surprises here and reduce triggers for re-realization by always saying we fully - * succeeded.) */ - if (ret_result_mask) - *ret_result_mask = mask & supported & CGROUP_MASK_V2; /* If you wonder why we mask this with - * CGROUP_MASK_V2: The 'supported' mask - * might contain pure-V1 or BPF - * controllers, and we never want to - * claim that we could enable those with - * cgroup.subtree_control */ - return 0; - } - - r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, p, "cgroup.subtree_control", &fs); - if (r < 0) - return r; - - for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { - CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); - const char *n; - - if (!FLAGS_SET(CGROUP_MASK_V2, bit)) - continue; - - if (!FLAGS_SET(supported, bit)) - continue; - - n = cgroup_controller_to_string(c); - { - char s[1 + strlen(n) + 1]; - - s[0] = FLAGS_SET(mask, bit) ? '+' : '-'; - strcpy(s + 1, n); - - if (!f) { - f = fopen(fs, "we"); - if (!f) - return log_debug_errno(errno, "Failed to open cgroup.subtree_control file of %s: %m", p); - } - - r = write_string_stream(f, s, WRITE_STRING_FILE_DISABLE_BUFFER); - if (r < 0) { - log_debug_errno(r, "Failed to %s controller %s for %s (%s): %m", - FLAGS_SET(mask, bit) ? "enable" : "disable", n, p, fs); - clearerr(f); - - /* If we can't turn off a controller, leave it on in the reported resulting mask. This - * happens for example when we attempt to turn off a controller up in the tree that is - * used down in the tree. */ - if (!FLAGS_SET(mask, bit) && r == -EBUSY) /* You might wonder why we check for EBUSY - * only here, and not follow the same logic - * for other errors such as EINVAL or - * EOPNOTSUPP or anything else. That's - * because EBUSY indicates that the - * controllers is currently enabled and - * cannot be disabled because something down - * the hierarchy is still using it. Any other - * error most likely means something like "I - * never heard of this controller" or - * similar. In the former case it's hence - * safe to assume the controller is still on - * after the failed operation, while in the - * latter case it's safer to assume the - * controller is unknown and hence certainly - * not enabled. */ - ret |= bit; - } else { - /* Otherwise, if we managed to turn on a controller, set the bit reflecting that. */ - if (FLAGS_SET(mask, bit)) - ret |= bit; - } - } - } - - /* Let's return the precise set of controllers now enabled for the cgroup. */ - if (ret_result_mask) - *ret_result_mask = ret; - - return 0; -} - -bool cg_is_unified_wanted(void) { - static thread_local int wanted = -1; - bool b; - const bool is_default = DEFAULT_HIERARCHY == CGROUP_UNIFIED_ALL; - _cleanup_free_ char *c = NULL; - int r; - - /* If we have a cached value, return that. */ - if (wanted >= 0) - return wanted; - - /* If the hierarchy is already mounted, then follow whatever - * was chosen for it. */ - r = cg_unified_cached(true); - if (r >= 0) - return (wanted = r >= CGROUP_UNIFIED_ALL); - - /* If we were explicitly passed systemd.unified_cgroup_hierarchy, - * respect that. */ - r = proc_cmdline_get_bool("systemd.unified_cgroup_hierarchy", &b); - if (r > 0) - return (wanted = b); - - /* If we passed cgroup_no_v1=all with no other instructions, it seems - * highly unlikely that we want to use hybrid or legacy hierarchy. */ - r = proc_cmdline_get_key("cgroup_no_v1", 0, &c); - if (r > 0 && streq_ptr(c, "all")) - return (wanted = true); - - return (wanted = is_default); -} - -bool cg_is_legacy_wanted(void) { - static thread_local int wanted = -1; - - /* If we have a cached value, return that. */ - if (wanted >= 0) - return wanted; - - /* Check if we have cgroup v2 already mounted. */ - if (cg_unified_cached(true) == CGROUP_UNIFIED_ALL) - return (wanted = false); - - /* Otherwise, assume that at least partial legacy is wanted, - * since cgroup v2 should already be mounted at this point. */ - return (wanted = true); -} - -bool cg_is_hybrid_wanted(void) { - static thread_local int wanted = -1; - int r; - bool b; - const bool is_default = DEFAULT_HIERARCHY >= CGROUP_UNIFIED_SYSTEMD; - /* We default to true if the default is "hybrid", obviously, - * but also when the default is "unified", because if we get - * called, it means that unified hierarchy was not mounted. */ - - /* If we have a cached value, return that. */ - if (wanted >= 0) - return wanted; - - /* If the hierarchy is already mounted, then follow whatever - * was chosen for it. */ - if (cg_unified_cached(true) == CGROUP_UNIFIED_ALL) - return (wanted = false); - - /* Otherwise, let's see what the kernel command line has to say. - * Since checking is expensive, cache a non-error result. */ - r = proc_cmdline_get_bool("systemd.legacy_systemd_cgroup_controller", &b); - - /* The meaning of the kernel option is reversed wrt. to the return value - * of this function, hence the negation. */ - return (wanted = r > 0 ? !b : is_default); -} - -int cg_weight_parse(const char *s, uint64_t *ret) { - uint64_t u; - int r; - - if (isempty(s)) { - *ret = CGROUP_WEIGHT_INVALID; - return 0; - } - - r = safe_atou64(s, &u); - if (r < 0) - return r; - - if (u < CGROUP_WEIGHT_MIN || u > CGROUP_WEIGHT_MAX) - return -ERANGE; - - *ret = u; - return 0; -} - const uint64_t cgroup_io_limit_defaults[_CGROUP_IO_LIMIT_TYPE_MAX] = { [CGROUP_IO_RBPS_MAX] = CGROUP_LIMIT_MAX, [CGROUP_IO_WBPS_MAX] = CGROUP_LIMIT_MAX, @@ -2845,46 +2039,6 @@ static const char* const cgroup_io_limit_type_table[_CGROUP_IO_LIMIT_TYPE_MAX] = DEFINE_STRING_TABLE_LOOKUP(cgroup_io_limit_type, CGroupIOLimitType); -int cg_cpu_shares_parse(const char *s, uint64_t *ret) { - uint64_t u; - int r; - - if (isempty(s)) { - *ret = CGROUP_CPU_SHARES_INVALID; - return 0; - } - - r = safe_atou64(s, &u); - if (r < 0) - return r; - - if (u < CGROUP_CPU_SHARES_MIN || u > CGROUP_CPU_SHARES_MAX) - return -ERANGE; - - *ret = u; - return 0; -} - -int cg_blkio_weight_parse(const char *s, uint64_t *ret) { - uint64_t u; - int r; - - if (isempty(s)) { - *ret = CGROUP_BLKIO_WEIGHT_INVALID; - return 0; - } - - r = safe_atou64(s, &u); - if (r < 0) - return r; - - if (u < CGROUP_BLKIO_WEIGHT_MIN || u > CGROUP_BLKIO_WEIGHT_MAX) - return -ERANGE; - - *ret = u; - return 0; -} - bool is_cgroup_fs(const struct statfs *s) { return is_fs_type(s, CGROUP_SUPER_MAGIC) || is_fs_type(s, CGROUP2_SUPER_MAGIC); diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index ba8df8139d..1472265204 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -172,10 +172,6 @@ typedef int (*cg_kill_log_func_t)(pid_t pid, int sig, void *userdata); int cg_kill(const char *controller, const char *path, int sig, CGroupFlags flags, Set *s, cg_kill_log_func_t kill_log, void *userdata); int cg_kill_recursive(const char *controller, const char *path, int sig, CGroupFlags flags, Set *s, cg_kill_log_func_t kill_log, void *userdata); -int cg_migrate(const char *cfrom, const char *pfrom, const char *cto, const char *pto, CGroupFlags flags); -int cg_migrate_recursive(const char *cfrom, const char *pfrom, const char *cto, const char *pto, CGroupFlags flags); -int cg_migrate_recursive_fallback(const char *cfrom, const char *pfrom, const char *cto, const char *pto, CGroupFlags flags); - int cg_split_spec(const char *spec, char **controller, char **path); int cg_mangle_path(const char *path, char **result); @@ -184,15 +180,8 @@ int cg_get_path_and_check(const char *controller, const char *path, const char * int cg_pid_get_path(const char *controller, pid_t pid, char **path); -int cg_trim(const char *controller, const char *path, bool delete_root); - int cg_rmdir(const char *controller, const char *path); -int cg_create(const char *controller, const char *path); -int cg_attach(const char *controller, const char *path, pid_t pid); -int cg_attach_fallback(const char *controller, const char *path, pid_t pid); -int cg_create_and_attach(const char *controller, const char *path, pid_t pid); - int cg_set_attribute(const char *controller, const char *path, const char *attribute, const char *value); int cg_get_attribute(const char *controller, const char *path, const char *attribute, char **ret); int cg_get_keyed_attribute(const char *controller, const char *path, const char *attribute, char **keys, char **values); @@ -240,13 +229,6 @@ int cg_slice_to_path(const char *unit, char **ret); typedef const char* (*cg_migrate_callback_t)(CGroupMask mask, void *userdata); -int cg_create_everywhere(CGroupMask supported, CGroupMask mask, const char *path); -int cg_attach_everywhere(CGroupMask supported, const char *path, pid_t pid, cg_migrate_callback_t callback, void *userdata); -int cg_attach_many_everywhere(CGroupMask supported, const char *path, Set* pids, cg_migrate_callback_t callback, void *userdata); -int cg_migrate_everywhere(CGroupMask supported, const char *from, const char *to, cg_migrate_callback_t callback, void *userdata); -int cg_trim_everywhere(CGroupMask supported, const char *path, bool delete_root); -int cg_enable_everywhere(CGroupMask supported, CGroupMask mask, const char *p, CGroupMask *ret_result_mask); - int cg_mask_supported(CGroupMask *ret); int cg_mask_from_string(const char *s, CGroupMask *ret); int cg_mask_to_string(CGroupMask mask, char **ret); @@ -263,16 +245,8 @@ static inline int cg_unified(void) { return cg_unified_cached(true); } -bool cg_is_unified_wanted(void); -bool cg_is_legacy_wanted(void); -bool cg_is_hybrid_wanted(void); - const char* cgroup_controller_to_string(CGroupController c) _const_; CGroupController cgroup_controller_from_string(const char *s) _pure_; -int cg_weight_parse(const char *s, uint64_t *ret); -int cg_cpu_shares_parse(const char *s, uint64_t *ret); -int cg_blkio_weight_parse(const char *s, uint64_t *ret); - bool is_cgroup_fs(const struct statfs *s); bool fd_is_cgroup_fs(int fd); diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 7b0a41fbc8..00690920cd 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -11,6 +11,7 @@ #include "bpf-firewall.h" #include "btrfs-util.h" #include "bus-error.h" +#include "cgroup-setup.h" #include "cgroup-util.h" #include "cgroup.h" #include "fd-util.h" diff --git a/src/core/execute.c b/src/core/execute.c index 4c90007778..f769d02be4 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -48,6 +48,7 @@ #include "cap-list.h" #include "capability-util.h" #include "chown-recursive.h" +#include "cgroup-setup.h" #include "cpu-set-util.h" #include "def.h" #include "env-file.h" diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index a7951a5757..e517c8dfbf 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -24,7 +24,7 @@ #include "bus-util.h" #include "cap-list.h" #include "capability-util.h" -#include "cgroup.h" +#include "cgroup-setup.h" #include "conf-parser.h" #include "cpu-set-util.h" #include "env-util.h" diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c index 72cf5a2caa..790f1e234e 100644 --- a/src/core/mount-setup.c +++ b/src/core/mount-setup.c @@ -11,6 +11,7 @@ #include "bus-util.h" #include "cgroup-util.h" #include "conf-files.h" +#include "cgroup-setup.h" #include "dev-setup.h" #include "efivars.h" #include "fd-util.h" diff --git a/src/core/unit.c b/src/core/unit.c index 52a1acafab..004d787d8f 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -15,6 +15,7 @@ #include "bpf-firewall.h" #include "bus-common-errors.h" #include "bus-util.h" +#include "cgroup-setup.h" #include "cgroup-util.h" #include "dbus-unit.h" #include "dbus.h" diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c index 3f762cbbc3..766d651c3f 100644 --- a/src/login/pam_systemd.c +++ b/src/login/pam_systemd.c @@ -20,7 +20,7 @@ #include "bus-error.h" #include "bus-internal.h" #include "bus-util.h" -#include "cgroup-util.h" +#include "cgroup-setup.h" #include "errno-util.h" #include "fd-util.h" #include "fileio.h" diff --git a/src/nspawn/nspawn-cgroup.c b/src/nspawn/nspawn-cgroup.c index 0462b46413..f5048d9473 100644 --- a/src/nspawn/nspawn-cgroup.c +++ b/src/nspawn/nspawn-cgroup.c @@ -3,6 +3,7 @@ #include #include "alloc-util.h" +#include "cgroup-setup.h" #include "fd-util.h" #include "fileio.h" #include "format-util.h" diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index e53b9d5ea2..286ef89878 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -5,6 +5,7 @@ #include "bus-unit-util.h" #include "bus-util.h" #include "cap-list.h" +#include "cgroup-setup.h" #include "cgroup-util.h" #include "condition.h" #include "cpu-set-util.h" diff --git a/src/shared/cgroup-setup.c b/src/shared/cgroup-setup.c new file mode 100644 index 0000000000..ddcd156801 --- /dev/null +++ b/src/shared/cgroup-setup.c @@ -0,0 +1,860 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +#include +#include + +#include "cgroup-setup.h" +#include "cgroup-util.h" +#include "errno-util.h" +#include "parse-util.h" +#include "path-util.h" +#include "proc-cmdline.h" +#include "stdio-util.h" +#include "string-util.h" +#include "fs-util.h" +#include "mkdir.h" +#include "process-util.h" +#include "fileio.h" +#include "user-util.h" +#include "fd-util.h" + +bool cg_is_unified_wanted(void) { + static thread_local int wanted = -1; + bool b; + const bool is_default = DEFAULT_HIERARCHY == CGROUP_UNIFIED_ALL; + _cleanup_free_ char *c = NULL; + int r; + + /* If we have a cached value, return that. */ + if (wanted >= 0) + return wanted; + + /* If the hierarchy is already mounted, then follow whatever was chosen for it. */ + r = cg_unified_cached(true); + if (r >= 0) + return (wanted = r >= CGROUP_UNIFIED_ALL); + + /* If we were explicitly passed systemd.unified_cgroup_hierarchy, respect that. */ + r = proc_cmdline_get_bool("systemd.unified_cgroup_hierarchy", &b); + if (r > 0) + return (wanted = b); + + /* If we passed cgroup_no_v1=all with no other instructions, it seems highly unlikely that we want to + * use hybrid or legacy hierarchy. */ + r = proc_cmdline_get_key("cgroup_no_v1", 0, &c); + if (r > 0 && streq_ptr(c, "all")) + return (wanted = true); + + return (wanted = is_default); +} + +bool cg_is_legacy_wanted(void) { + static thread_local int wanted = -1; + + /* If we have a cached value, return that. */ + if (wanted >= 0) + return wanted; + + /* Check if we have cgroup v2 already mounted. */ + if (cg_unified_cached(true) == CGROUP_UNIFIED_ALL) + return (wanted = false); + + /* Otherwise, assume that at least partial legacy is wanted, + * since cgroup v2 should already be mounted at this point. */ + return (wanted = true); +} + +bool cg_is_hybrid_wanted(void) { + static thread_local int wanted = -1; + int r; + bool b; + const bool is_default = DEFAULT_HIERARCHY >= CGROUP_UNIFIED_SYSTEMD; + /* We default to true if the default is "hybrid", obviously, but also when the default is "unified", + * because if we get called, it means that unified hierarchy was not mounted. */ + + /* If we have a cached value, return that. */ + if (wanted >= 0) + return wanted; + + /* If the hierarchy is already mounted, then follow whatever was chosen for it. */ + if (cg_unified_cached(true) == CGROUP_UNIFIED_ALL) + return (wanted = false); + + /* Otherwise, let's see what the kernel command line has to say. Since checking is expensive, cache + * a non-error result. */ + r = proc_cmdline_get_bool("systemd.legacy_systemd_cgroup_controller", &b); + + /* The meaning of the kernel option is reversed wrt. to the return value of this function, hence the + * negation. */ + return (wanted = r > 0 ? !b : is_default); +} + +int cg_weight_parse(const char *s, uint64_t *ret) { + uint64_t u; + int r; + + if (isempty(s)) { + *ret = CGROUP_WEIGHT_INVALID; + return 0; + } + + r = safe_atou64(s, &u); + if (r < 0) + return r; + + if (u < CGROUP_WEIGHT_MIN || u > CGROUP_WEIGHT_MAX) + return -ERANGE; + + *ret = u; + return 0; +} + +int cg_cpu_shares_parse(const char *s, uint64_t *ret) { + uint64_t u; + int r; + + if (isempty(s)) { + *ret = CGROUP_CPU_SHARES_INVALID; + return 0; + } + + r = safe_atou64(s, &u); + if (r < 0) + return r; + + if (u < CGROUP_CPU_SHARES_MIN || u > CGROUP_CPU_SHARES_MAX) + return -ERANGE; + + *ret = u; + return 0; +} + +int cg_blkio_weight_parse(const char *s, uint64_t *ret) { + uint64_t u; + int r; + + if (isempty(s)) { + *ret = CGROUP_BLKIO_WEIGHT_INVALID; + return 0; + } + + r = safe_atou64(s, &u); + if (r < 0) + return r; + + if (u < CGROUP_BLKIO_WEIGHT_MIN || u > CGROUP_BLKIO_WEIGHT_MAX) + return -ERANGE; + + *ret = u; + return 0; +} + + +static int trim_cb(const char *path, const struct stat *sb, int typeflag, struct FTW *ftwbuf) { + assert(path); + assert(sb); + assert(ftwbuf); + + if (typeflag != FTW_DP) + return 0; + + if (ftwbuf->level < 1) + return 0; + + (void) rmdir(path); + return 0; +} + +int cg_trim(const char *controller, const char *path, bool delete_root) { + _cleanup_free_ char *fs = NULL; + int r = 0, q; + + assert(path); + + r = cg_get_path(controller, path, NULL, &fs); + if (r < 0) + return r; + + errno = 0; + if (nftw(fs, trim_cb, 64, FTW_DEPTH|FTW_MOUNT|FTW_PHYS) != 0) { + if (errno == ENOENT) + r = 0; + else + r = errno_or_else(EIO); + } + + if (delete_root) { + if (rmdir(fs) < 0 && errno != ENOENT) + return -errno; + } + + q = cg_hybrid_unified(); + if (q < 0) + return q; + if (q > 0 && streq(controller, SYSTEMD_CGROUP_CONTROLLER)) { + q = cg_trim(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, delete_root); + if (q < 0) + log_warning_errno(q, "Failed to trim compat systemd cgroup %s: %m", path); + } + + return r; +} + +/* Create a cgroup in the hierarchy of controller. + * Returns 0 if the group already existed, 1 on success, negative otherwise. + */ +int cg_create(const char *controller, const char *path) { + _cleanup_free_ char *fs = NULL; + int r; + + r = cg_get_path_and_check(controller, path, NULL, &fs); + if (r < 0) + return r; + + r = mkdir_parents(fs, 0755); + if (r < 0) + return r; + + r = mkdir_errno_wrapper(fs, 0755); + if (r == -EEXIST) + return 0; + if (r < 0) + return r; + + r = cg_hybrid_unified(); + if (r < 0) + return r; + + if (r > 0 && streq(controller, SYSTEMD_CGROUP_CONTROLLER)) { + r = cg_create(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path); + if (r < 0) + log_warning_errno(r, "Failed to create compat systemd cgroup %s: %m", path); + } + + return 1; +} + +int cg_create_and_attach(const char *controller, const char *path, pid_t pid) { + int r, q; + + assert(pid >= 0); + + r = cg_create(controller, path); + if (r < 0) + return r; + + q = cg_attach(controller, path, pid); + if (q < 0) + return q; + + /* This does not remove the cgroup on failure */ + return r; +} + +int cg_attach(const char *controller, const char *path, pid_t pid) { + _cleanup_free_ char *fs = NULL; + char c[DECIMAL_STR_MAX(pid_t) + 2]; + int r; + + assert(path); + assert(pid >= 0); + + r = cg_get_path_and_check(controller, path, "cgroup.procs", &fs); + if (r < 0) + return r; + + if (pid == 0) + pid = getpid_cached(); + + xsprintf(c, PID_FMT "\n", pid); + + r = write_string_file(fs, c, WRITE_STRING_FILE_DISABLE_BUFFER); + if (r < 0) + return r; + + r = cg_hybrid_unified(); + if (r < 0) + return r; + + if (r > 0 && streq(controller, SYSTEMD_CGROUP_CONTROLLER)) { + r = cg_attach(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, pid); + if (r < 0) + log_warning_errno(r, "Failed to attach "PID_FMT" to compat systemd cgroup %s: %m", pid, path); + } + + return 0; +} + +int cg_attach_fallback(const char *controller, const char *path, pid_t pid) { + int r; + + assert(controller); + assert(path); + assert(pid >= 0); + + r = cg_attach(controller, path, pid); + if (r < 0) { + char prefix[strlen(path) + 1]; + + /* This didn't work? Then let's try all prefixes of + * the destination */ + + PATH_FOREACH_PREFIX(prefix, path) { + int q; + + q = cg_attach(controller, prefix, pid); + if (q >= 0) + return q; + } + } + + return r; +} + +int cg_set_access( + const char *controller, + const char *path, + uid_t uid, + gid_t gid) { + + struct Attribute { + const char *name; + bool fatal; + }; + + /* cgroup v1, aka legacy/non-unified */ + static const struct Attribute legacy_attributes[] = { + { "cgroup.procs", true }, + { "tasks", false }, + { "cgroup.clone_children", false }, + {}, + }; + + /* cgroup v2, aka unified */ + static const struct Attribute unified_attributes[] = { + { "cgroup.procs", true }, + { "cgroup.subtree_control", true }, + { "cgroup.threads", false }, + {}, + }; + + static const struct Attribute* const attributes[] = { + [false] = legacy_attributes, + [true] = unified_attributes, + }; + + _cleanup_free_ char *fs = NULL; + const struct Attribute *i; + int r, unified; + + assert(path); + + if (uid == UID_INVALID && gid == GID_INVALID) + return 0; + + unified = cg_unified_controller(controller); + if (unified < 0) + return unified; + + /* Configure access to the cgroup itself */ + r = cg_get_path(controller, path, NULL, &fs); + if (r < 0) + return r; + + r = chmod_and_chown(fs, 0755, uid, gid); + if (r < 0) + return r; + + /* Configure access to the cgroup's attributes */ + for (i = attributes[unified]; i->name; i++) { + fs = mfree(fs); + + r = cg_get_path(controller, path, i->name, &fs); + if (r < 0) + return r; + + r = chmod_and_chown(fs, 0644, uid, gid); + if (r < 0) { + if (i->fatal) + return r; + + log_debug_errno(r, "Failed to set access on cgroup %s, ignoring: %m", fs); + } + } + + if (streq(controller, SYSTEMD_CGROUP_CONTROLLER)) { + r = cg_hybrid_unified(); + if (r < 0) + return r; + if (r > 0) { + /* Always propagate access mode from unified to legacy controller */ + r = cg_set_access(SYSTEMD_CGROUP_CONTROLLER_LEGACY, path, uid, gid); + if (r < 0) + log_debug_errno(r, "Failed to set access on compatibility systemd cgroup %s, ignoring: %m", path); + } + } + + return 0; +} + +int cg_migrate( + const char *cfrom, + const char *pfrom, + const char *cto, + const char *pto, + CGroupFlags flags) { + + bool done = false; + _cleanup_set_free_ Set *s = NULL; + int r, ret = 0; + pid_t my_pid; + + assert(cfrom); + assert(pfrom); + assert(cto); + assert(pto); + + s = set_new(NULL); + if (!s) + return -ENOMEM; + + my_pid = getpid_cached(); + + do { + _cleanup_fclose_ FILE *f = NULL; + pid_t pid = 0; + done = true; + + r = cg_enumerate_processes(cfrom, pfrom, &f); + if (r < 0) { + if (ret >= 0 && r != -ENOENT) + return r; + + return ret; + } + + while ((r = cg_read_pid(f, &pid)) > 0) { + + /* This might do weird stuff if we aren't a + * single-threaded program. However, we + * luckily know we are not */ + if ((flags & CGROUP_IGNORE_SELF) && pid == my_pid) + continue; + + if (set_get(s, PID_TO_PTR(pid)) == PID_TO_PTR(pid)) + continue; + + /* Ignore kernel threads. Since they can only + * exist in the root cgroup, we only check for + * them there. */ + if (cfrom && + empty_or_root(pfrom) && + is_kernel_thread(pid) > 0) + continue; + + r = cg_attach(cto, pto, pid); + if (r < 0) { + if (ret >= 0 && r != -ESRCH) + ret = r; + } else if (ret == 0) + ret = 1; + + done = false; + + r = set_put(s, PID_TO_PTR(pid)); + if (r < 0) { + if (ret >= 0) + return r; + + return ret; + } + } + + if (r < 0) { + if (ret >= 0) + return r; + + return ret; + } + } while (!done); + + return ret; +} + +int cg_migrate_recursive( + const char *cfrom, + const char *pfrom, + const char *cto, + const char *pto, + CGroupFlags flags) { + + _cleanup_closedir_ DIR *d = NULL; + int r, ret = 0; + char *fn; + + assert(cfrom); + assert(pfrom); + assert(cto); + assert(pto); + + ret = cg_migrate(cfrom, pfrom, cto, pto, flags); + + r = cg_enumerate_subgroups(cfrom, pfrom, &d); + if (r < 0) { + if (ret >= 0 && r != -ENOENT) + return r; + + return ret; + } + + while ((r = cg_read_subgroup(d, &fn)) > 0) { + _cleanup_free_ char *p = NULL; + + p = path_join(empty_to_root(pfrom), fn); + free(fn); + if (!p) + return -ENOMEM; + + r = cg_migrate_recursive(cfrom, p, cto, pto, flags); + if (r != 0 && ret >= 0) + ret = r; + } + + if (r < 0 && ret >= 0) + ret = r; + + if (flags & CGROUP_REMOVE) { + r = cg_rmdir(cfrom, pfrom); + if (r < 0 && ret >= 0 && !IN_SET(r, -ENOENT, -EBUSY)) + return r; + } + + return ret; +} + +int cg_migrate_recursive_fallback( + const char *cfrom, + const char *pfrom, + const char *cto, + const char *pto, + CGroupFlags flags) { + + int r; + + assert(cfrom); + assert(pfrom); + assert(cto); + assert(pto); + + r = cg_migrate_recursive(cfrom, pfrom, cto, pto, flags); + if (r < 0) { + char prefix[strlen(pto) + 1]; + + /* This didn't work? Then let's try all prefixes of the destination */ + + PATH_FOREACH_PREFIX(prefix, pto) { + int q; + + q = cg_migrate_recursive(cfrom, pfrom, cto, prefix, flags); + if (q >= 0) + return q; + } + } + + return r; +} + +int cg_create_everywhere(CGroupMask supported, CGroupMask mask, const char *path) { + CGroupController c; + CGroupMask done; + bool created; + int r; + + /* This one will create a cgroup in our private tree, but also + * duplicate it in the trees specified in mask, and remove it + * in all others. + * + * Returns 0 if the group already existed in the systemd hierarchy, + * 1 on success, negative otherwise. + */ + + /* First create the cgroup in our own hierarchy. */ + r = cg_create(SYSTEMD_CGROUP_CONTROLLER, path); + if (r < 0) + return r; + created = r; + + /* If we are in the unified hierarchy, we are done now */ + r = cg_all_unified(); + if (r < 0) + return r; + if (r > 0) + return created; + + supported &= CGROUP_MASK_V1; + mask = CGROUP_MASK_EXTEND_JOINED(mask); + done = 0; + + /* Otherwise, do the same in the other hierarchies */ + for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { + CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); + const char *n; + + if (!FLAGS_SET(supported, bit)) + continue; + + if (FLAGS_SET(done, bit)) + continue; + + n = cgroup_controller_to_string(c); + if (FLAGS_SET(mask, bit)) + (void) cg_create(n, path); + else + (void) cg_trim(n, path, true); + + done |= CGROUP_MASK_EXTEND_JOINED(bit); + } + + return created; +} + +int cg_attach_everywhere(CGroupMask supported, const char *path, pid_t pid, cg_migrate_callback_t path_callback, void *userdata) { + CGroupController c; + CGroupMask done; + int r; + + r = cg_attach(SYSTEMD_CGROUP_CONTROLLER, path, pid); + if (r < 0) + return r; + + r = cg_all_unified(); + if (r < 0) + return r; + if (r > 0) + return 0; + + supported &= CGROUP_MASK_V1; + done = 0; + + for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { + CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); + const char *p = NULL; + + if (!FLAGS_SET(supported, bit)) + continue; + + if (FLAGS_SET(done, bit)) + continue; + + if (path_callback) + p = path_callback(bit, userdata); + if (!p) + p = path; + + (void) cg_attach_fallback(cgroup_controller_to_string(c), p, pid); + done |= CGROUP_MASK_EXTEND_JOINED(bit); + } + + return 0; +} + +int cg_attach_many_everywhere(CGroupMask supported, const char *path, Set* pids, cg_migrate_callback_t path_callback, void *userdata) { + Iterator i; + void *pidp; + int r = 0; + + SET_FOREACH(pidp, pids, i) { + pid_t pid = PTR_TO_PID(pidp); + int q; + + q = cg_attach_everywhere(supported, path, pid, path_callback, userdata); + if (q < 0 && r >= 0) + r = q; + } + + return r; +} + +int cg_migrate_everywhere(CGroupMask supported, const char *from, const char *to, cg_migrate_callback_t to_callback, void *userdata) { + CGroupController c; + CGroupMask done; + int r = 0, q; + + if (!path_equal(from, to)) { + r = cg_migrate_recursive(SYSTEMD_CGROUP_CONTROLLER, from, SYSTEMD_CGROUP_CONTROLLER, to, CGROUP_REMOVE); + if (r < 0) + return r; + } + + q = cg_all_unified(); + if (q < 0) + return q; + if (q > 0) + return r; + + supported &= CGROUP_MASK_V1; + done = 0; + + for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { + CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); + const char *p = NULL; + + if (!FLAGS_SET(supported, bit)) + continue; + + if (FLAGS_SET(done, bit)) + continue; + + if (to_callback) + p = to_callback(bit, userdata); + if (!p) + p = to; + + (void) cg_migrate_recursive_fallback(SYSTEMD_CGROUP_CONTROLLER, to, cgroup_controller_to_string(c), p, 0); + done |= CGROUP_MASK_EXTEND_JOINED(bit); + } + + return r; +} + +int cg_trim_everywhere(CGroupMask supported, const char *path, bool delete_root) { + CGroupController c; + CGroupMask done; + int r, q; + + r = cg_trim(SYSTEMD_CGROUP_CONTROLLER, path, delete_root); + if (r < 0) + return r; + + q = cg_all_unified(); + if (q < 0) + return q; + if (q > 0) + return r; + + supported &= CGROUP_MASK_V1; + done = 0; + + for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { + CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); + + if (!FLAGS_SET(supported, bit)) + continue; + + if (FLAGS_SET(done, bit)) + continue; + + (void) cg_trim(cgroup_controller_to_string(c), path, delete_root); + done |= CGROUP_MASK_EXTEND_JOINED(bit); + } + + return r; +} + +int cg_enable_everywhere( + CGroupMask supported, + CGroupMask mask, + const char *p, + CGroupMask *ret_result_mask) { + + _cleanup_fclose_ FILE *f = NULL; + _cleanup_free_ char *fs = NULL; + CGroupController c; + CGroupMask ret = 0; + int r; + + assert(p); + + if (supported == 0) { + if (ret_result_mask) + *ret_result_mask = 0; + return 0; + } + + r = cg_all_unified(); + if (r < 0) + return r; + if (r == 0) { + /* On the legacy hierarchy there's no concept of "enabling" controllers in cgroups defined. Let's claim + * complete success right away. (If you wonder why we return the full mask here, rather than zero: the + * caller tends to use the returned mask later on to compare if all controllers where properly joined, + * and if not requeues realization. This use is the primary purpose of the return value, hence let's + * minimize surprises here and reduce triggers for re-realization by always saying we fully + * succeeded.) */ + if (ret_result_mask) + *ret_result_mask = mask & supported & CGROUP_MASK_V2; /* If you wonder why we mask this with + * CGROUP_MASK_V2: The 'supported' mask + * might contain pure-V1 or BPF + * controllers, and we never want to + * claim that we could enable those with + * cgroup.subtree_control */ + return 0; + } + + r = cg_get_path(SYSTEMD_CGROUP_CONTROLLER, p, "cgroup.subtree_control", &fs); + if (r < 0) + return r; + + for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { + CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); + const char *n; + + if (!FLAGS_SET(CGROUP_MASK_V2, bit)) + continue; + + if (!FLAGS_SET(supported, bit)) + continue; + + n = cgroup_controller_to_string(c); + { + char s[1 + strlen(n) + 1]; + + s[0] = FLAGS_SET(mask, bit) ? '+' : '-'; + strcpy(s + 1, n); + + if (!f) { + f = fopen(fs, "we"); + if (!f) + return log_debug_errno(errno, "Failed to open cgroup.subtree_control file of %s: %m", p); + } + + r = write_string_stream(f, s, WRITE_STRING_FILE_DISABLE_BUFFER); + if (r < 0) { + log_debug_errno(r, "Failed to %s controller %s for %s (%s): %m", + FLAGS_SET(mask, bit) ? "enable" : "disable", n, p, fs); + clearerr(f); + + /* If we can't turn off a controller, leave it on in the reported resulting mask. This + * happens for example when we attempt to turn off a controller up in the tree that is + * used down in the tree. */ + if (!FLAGS_SET(mask, bit) && r == -EBUSY) /* You might wonder why we check for EBUSY + * only here, and not follow the same logic + * for other errors such as EINVAL or + * EOPNOTSUPP or anything else. That's + * because EBUSY indicates that the + * controllers is currently enabled and + * cannot be disabled because something down + * the hierarchy is still using it. Any other + * error most likely means something like "I + * never heard of this controller" or + * similar. In the former case it's hence + * safe to assume the controller is still on + * after the failed operation, while in the + * latter case it's safer to assume the + * controller is unknown and hence certainly + * not enabled. */ + ret |= bit; + } else { + /* Otherwise, if we managed to turn on a controller, set the bit reflecting that. */ + if (FLAGS_SET(mask, bit)) + ret |= bit; + } + } + } + + /* Let's return the precise set of controllers now enabled for the cgroup. */ + if (ret_result_mask) + *ret_result_mask = ret; + + return 0; +} diff --git a/src/shared/cgroup-setup.h b/src/shared/cgroup-setup.h new file mode 100644 index 0000000000..6e9b6857d8 --- /dev/null +++ b/src/shared/cgroup-setup.h @@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ +#pragma once + +#include +#include +#include + +#include "cgroup-util.h" + +bool cg_is_unified_wanted(void); +bool cg_is_legacy_wanted(void); +bool cg_is_hybrid_wanted(void); + +int cg_weight_parse(const char *s, uint64_t *ret); +int cg_cpu_shares_parse(const char *s, uint64_t *ret); +int cg_blkio_weight_parse(const char *s, uint64_t *ret); + +int cg_trim(const char *controller, const char *path, bool delete_root); + +int cg_create(const char *controller, const char *path); +int cg_attach(const char *controller, const char *path, pid_t pid); +int cg_attach_fallback(const char *controller, const char *path, pid_t pid); +int cg_create_and_attach(const char *controller, const char *path, pid_t pid); + +int cg_migrate(const char *cfrom, const char *pfrom, const char *cto, const char *pto, CGroupFlags flags); +int cg_migrate_recursive(const char *cfrom, const char *pfrom, const char *cto, const char *pto, CGroupFlags flags); +int cg_migrate_recursive_fallback(const char *cfrom, const char *pfrom, const char *cto, const char *pto, CGroupFlags flags); + +int cg_create_everywhere(CGroupMask supported, CGroupMask mask, const char *path); +int cg_attach_everywhere(CGroupMask supported, const char *path, pid_t pid, cg_migrate_callback_t callback, void *userdata); +int cg_attach_many_everywhere(CGroupMask supported, const char *path, Set* pids, cg_migrate_callback_t callback, void *userdata); +int cg_migrate_everywhere(CGroupMask supported, const char *from, const char *to, cg_migrate_callback_t callback, void *userdata); +int cg_trim_everywhere(CGroupMask supported, const char *path, bool delete_root); +int cg_enable_everywhere(CGroupMask supported, CGroupMask mask, const char *p, CGroupMask *ret_result_mask); diff --git a/src/shared/meson.build b/src/shared/meson.build index e9005a30e3..63a3f88d50 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -33,6 +33,8 @@ shared_sources = files(''' bus-wait-for-units.h calendarspec.c calendarspec.h + cgroup-setup.c + cgroup-setup.h cgroup-show.c cgroup-show.h clean-ipc.c diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index 0eb17989d0..08215fd3ee 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -17,6 +17,7 @@ #include "alloc-util.h" #include "async.h" +#include "cgroup-setup.h" #include "cgroup-util.h" #include "def.h" #include "exec-util.h" diff --git a/src/test/meson.build b/src/test/meson.build index a8c3e59098..5132145b41 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -615,6 +615,10 @@ tests += [ [], []], + [['src/test/test-cgroup-setup.c'], + [], + []], + [['src/test/test-env-file.c'], [], []], diff --git a/src/test/test-cgroup-setup.c b/src/test/test-cgroup-setup.c new file mode 100644 index 0000000000..330631a910 --- /dev/null +++ b/src/test/test-cgroup-setup.c @@ -0,0 +1,67 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +#include "alloc-util.h" +#include "build.h" +#include "cgroup-setup.h" +#include "log.h" +#include "proc-cmdline.h" +#include "string-util.h" +#include "tests.h" + +static void test_is_wanted_print(bool header) { + _cleanup_free_ char *cmdline = NULL; + + log_info("-- %s --", __func__); + assert_se(proc_cmdline(&cmdline) >= 0); + log_info("cmdline: %s", cmdline); + if (header) { + log_info(_CGROUP_HIERARCHY_); + (void) system("findmnt -n /sys/fs/cgroup"); + } + + log_info("is_unified_wanted() → %s", yes_no(cg_is_unified_wanted())); + log_info("is_hybrid_wanted() → %s", yes_no(cg_is_hybrid_wanted())); + log_info("is_legacy_wanted() → %s", yes_no(cg_is_legacy_wanted())); + log_info(" "); +} + +static void test_is_wanted(void) { + assert_se(setenv("SYSTEMD_PROC_CMDLINE", + "systemd.unified_cgroup_hierarchy", 1) >= 0); + test_is_wanted_print(false); + + assert_se(setenv("SYSTEMD_PROC_CMDLINE", + "systemd.unified_cgroup_hierarchy=0", 1) >= 0); + test_is_wanted_print(false); + + assert_se(setenv("SYSTEMD_PROC_CMDLINE", + "systemd.unified_cgroup_hierarchy=0 " + "systemd.legacy_systemd_cgroup_controller", 1) >= 0); + test_is_wanted_print(false); + + assert_se(setenv("SYSTEMD_PROC_CMDLINE", + "systemd.unified_cgroup_hierarchy=0 " + "systemd.legacy_systemd_cgroup_controller=0", 1) >= 0); + test_is_wanted_print(false); + + /* cgroup_no_v1=all implies unified cgroup hierarchy, unless otherwise + * explicitly specified. */ + assert_se(setenv("SYSTEMD_PROC_CMDLINE", + "cgroup_no_v1=all", 1) >= 0); + test_is_wanted_print(false); + + assert_se(setenv("SYSTEMD_PROC_CMDLINE", + "cgroup_no_v1=all " + "systemd.unified_cgroup_hierarchy=0", 1) >= 0); + test_is_wanted_print(false); +} + +int main(void) { + test_setup_logging(LOG_DEBUG); + + test_is_wanted_print(true); + test_is_wanted_print(false); /* run twice to test caching */ + test_is_wanted(); + + return 0; +} diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c index f45c5ff760..e4fbb9edce 100644 --- a/src/test/test-cgroup-util.c +++ b/src/test/test-cgroup-util.c @@ -333,55 +333,6 @@ static void test_fd_is_cgroup_fs(void) { fd = safe_close(fd); } -static void test_is_wanted_print(bool header) { - _cleanup_free_ char *cmdline = NULL; - - log_info("-- %s --", __func__); - assert_se(proc_cmdline(&cmdline) >= 0); - log_info("cmdline: %s", cmdline); - if (header) { - - log_info(_CGROUP_HIERARCHY_); - (void) system("findmnt -n /sys/fs/cgroup"); - } - - log_info("is_unified_wanted() → %s", yes_no(cg_is_unified_wanted())); - log_info("is_hybrid_wanted() → %s", yes_no(cg_is_hybrid_wanted())); - log_info("is_legacy_wanted() → %s", yes_no(cg_is_legacy_wanted())); - log_info(" "); -} - -static void test_is_wanted(void) { - assert_se(setenv("SYSTEMD_PROC_CMDLINE", - "systemd.unified_cgroup_hierarchy", 1) >= 0); - test_is_wanted_print(false); - - assert_se(setenv("SYSTEMD_PROC_CMDLINE", - "systemd.unified_cgroup_hierarchy=0", 1) >= 0); - test_is_wanted_print(false); - - assert_se(setenv("SYSTEMD_PROC_CMDLINE", - "systemd.unified_cgroup_hierarchy=0 " - "systemd.legacy_systemd_cgroup_controller", 1) >= 0); - test_is_wanted_print(false); - - assert_se(setenv("SYSTEMD_PROC_CMDLINE", - "systemd.unified_cgroup_hierarchy=0 " - "systemd.legacy_systemd_cgroup_controller=0", 1) >= 0); - test_is_wanted_print(false); - - /* cgroup_no_v1=all implies unified cgroup hierarchy, unless otherwise - * explicitly specified. */ - assert_se(setenv("SYSTEMD_PROC_CMDLINE", - "cgroup_no_v1=all", 1) >= 0); - test_is_wanted_print(false); - - assert_se(setenv("SYSTEMD_PROC_CMDLINE", - "cgroup_no_v1=all " - "systemd.unified_cgroup_hierarchy=0", 1) >= 0); - test_is_wanted_print(false); -} - static void test_cg_tests(void) { int all, hybrid, systemd, r; @@ -477,9 +428,6 @@ int main(void) { TEST_REQ_RUNNING_SYSTEMD(test_mask_supported()); TEST_REQ_RUNNING_SYSTEMD(test_is_cgroup_fs()); TEST_REQ_RUNNING_SYSTEMD(test_fd_is_cgroup_fs()); - test_is_wanted_print(true); - test_is_wanted_print(false); /* run twice to test caching */ - test_is_wanted(); test_cg_tests(); test_cg_get_keyed_attribute(); diff --git a/src/test/test-cgroup.c b/src/test/test-cgroup.c index 5cdfd2dc54..1891df0eb9 100644 --- a/src/test/test-cgroup.c +++ b/src/test/test-cgroup.c @@ -3,6 +3,7 @@ #include #include +#include "cgroup-setup.h" #include "cgroup-util.h" #include "path-util.h" #include "process-util.h" diff --git a/src/test/test-helper.c b/src/test/test-helper.c index 5b79d12f07..dc8c80a14b 100644 --- a/src/test/test-helper.c +++ b/src/test/test-helper.c @@ -3,7 +3,7 @@ #include "test-helper.h" #include "random-util.h" #include "alloc-util.h" -#include "cgroup-util.h" +#include "cgroup-setup.h" #include "string-util.h" int enter_cgroup_subroot(void) { From da33cba0f8cf3d4c23c11218485d8d4f339fc45b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Aug 2019 14:25:14 +0200 Subject: [PATCH 04/10] util-lib: move runlevel_to_target() to shared/unit-file It if of course related to /proc/cmdline parsing, but is higher-level functionality built on top of it. It should be in shared/ because it is something to be used by pid1 and related utilities, not something for level-level libraries. --- src/basic/proc-cmdline.c | 44 ------------------ src/basic/proc-cmdline.h | 1 - src/debug-generator/debug-generator.c | 1 + src/shared/unit-file.c | 45 +++++++++++++++++++ src/shared/unit-file.h | 2 + .../system-update-generator.c | 1 + src/test/test-proc-cmdline.c | 19 -------- src/test/test-unit-file.c | 20 +++++++++ 8 files changed, 69 insertions(+), 64 deletions(-) diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index 09169cf963..5d29774e09 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -317,47 +317,3 @@ int shall_restore_state(void) { return r > 0 ? ret : true; } - -static const char * const rlmap[] = { - "emergency", SPECIAL_EMERGENCY_TARGET, - "-b", SPECIAL_EMERGENCY_TARGET, - "rescue", SPECIAL_RESCUE_TARGET, - "single", SPECIAL_RESCUE_TARGET, - "-s", SPECIAL_RESCUE_TARGET, - "s", SPECIAL_RESCUE_TARGET, - "S", SPECIAL_RESCUE_TARGET, - "1", SPECIAL_RESCUE_TARGET, - "2", SPECIAL_MULTI_USER_TARGET, - "3", SPECIAL_MULTI_USER_TARGET, - "4", SPECIAL_MULTI_USER_TARGET, - "5", SPECIAL_GRAPHICAL_TARGET, - NULL -}; - -static const char * const rlmap_initrd[] = { - "emergency", SPECIAL_EMERGENCY_TARGET, - "rescue", SPECIAL_RESCUE_TARGET, - NULL -}; - -const char* runlevel_to_target(const char *word) { - const char * const *rlmap_ptr; - size_t i; - - if (!word) - return NULL; - - if (in_initrd()) { - word = startswith(word, "rd."); - if (!word) - return NULL; - } - - rlmap_ptr = in_initrd() ? rlmap_initrd : rlmap; - - for (i = 0; rlmap_ptr[i]; i += 2) - if (streq(word, rlmap_ptr[i])) - return rlmap_ptr[i+1]; - - return NULL; -} diff --git a/src/basic/proc-cmdline.h b/src/basic/proc-cmdline.h index ff04379fbd..429e7ca4a4 100644 --- a/src/basic/proc-cmdline.h +++ b/src/basic/proc-cmdline.h @@ -28,7 +28,6 @@ char *proc_cmdline_key_startswith(const char *s, const char *prefix); bool proc_cmdline_key_streq(const char *x, const char *y); int shall_restore_state(void); -const char* runlevel_to_target(const char *rl); /* A little helper call, to be used in proc_cmdline_parse_t callbacks */ static inline bool proc_cmdline_value_missing(const char *key, const char *value) { diff --git a/src/debug-generator/debug-generator.c b/src/debug-generator/debug-generator.c index e73dde32b8..bc8714c4c7 100644 --- a/src/debug-generator/debug-generator.c +++ b/src/debug-generator/debug-generator.c @@ -12,6 +12,7 @@ #include "special.h" #include "string-util.h" #include "strv.h" +#include "unit-file.h" #include "unit-name.h" static const char *arg_dest = NULL; diff --git a/src/shared/unit-file.c b/src/shared/unit-file.c index 4a5f23e6c1..9bcd5dc92e 100644 --- a/src/shared/unit-file.c +++ b/src/shared/unit-file.c @@ -6,6 +6,7 @@ #include "macro.h" #include "path-lookup.h" #include "set.h" +#include "special.h" #include "stat-util.h" #include "string-util.h" #include "strv.h" @@ -511,3 +512,47 @@ int unit_file_find_fragment( // FIXME: if instance, consider any unit names with different template name return 0; } + +static const char * const rlmap[] = { + "emergency", SPECIAL_EMERGENCY_TARGET, + "-b", SPECIAL_EMERGENCY_TARGET, + "rescue", SPECIAL_RESCUE_TARGET, + "single", SPECIAL_RESCUE_TARGET, + "-s", SPECIAL_RESCUE_TARGET, + "s", SPECIAL_RESCUE_TARGET, + "S", SPECIAL_RESCUE_TARGET, + "1", SPECIAL_RESCUE_TARGET, + "2", SPECIAL_MULTI_USER_TARGET, + "3", SPECIAL_MULTI_USER_TARGET, + "4", SPECIAL_MULTI_USER_TARGET, + "5", SPECIAL_GRAPHICAL_TARGET, + NULL +}; + +static const char * const rlmap_initrd[] = { + "emergency", SPECIAL_EMERGENCY_TARGET, + "rescue", SPECIAL_RESCUE_TARGET, + NULL +}; + +const char* runlevel_to_target(const char *word) { + const char * const *rlmap_ptr; + size_t i; + + if (!word) + return NULL; + + if (in_initrd()) { + word = startswith(word, "rd."); + if (!word) + return NULL; + } + + rlmap_ptr = in_initrd() ? rlmap_initrd : rlmap; + + for (i = 0; rlmap_ptr[i]; i += 2) + if (streq(word, rlmap_ptr[i])) + return rlmap_ptr[i+1]; + + return NULL; +} diff --git a/src/shared/unit-file.h b/src/shared/unit-file.h index 54cc7876fe..98ba677f3f 100644 --- a/src/shared/unit-file.h +++ b/src/shared/unit-file.h @@ -54,3 +54,5 @@ int unit_file_find_fragment( const char *unit_name, const char **ret_fragment_path, Set **names); + +const char* runlevel_to_target(const char *rl); diff --git a/src/system-update-generator/system-update-generator.c b/src/system-update-generator/system-update-generator.c index 6ec4986c10..666affca19 100644 --- a/src/system-update-generator/system-update-generator.c +++ b/src/system-update-generator/system-update-generator.c @@ -9,6 +9,7 @@ #include "proc-cmdline.h" #include "special.h" #include "string-util.h" +#include "unit-file.h" #include "util.h" /* diff --git a/src/test/test-proc-cmdline.c b/src/test/test-proc-cmdline.c index 6d25a6919e..884fb36010 100644 --- a/src/test/test-proc-cmdline.c +++ b/src/test/test-proc-cmdline.c @@ -212,24 +212,6 @@ static void test_proc_cmdline_key_startswith(void) { assert_se(!proc_cmdline_key_startswith("foo-bar", "foo_xx")); } -static void test_runlevel_to_target(void) { - log_info("/* %s */", __func__); - - in_initrd_force(false); - assert_se(streq_ptr(runlevel_to_target(NULL), NULL)); - assert_se(streq_ptr(runlevel_to_target("unknown-runlevel"), NULL)); - assert_se(streq_ptr(runlevel_to_target("rd.unknown-runlevel"), NULL)); - assert_se(streq_ptr(runlevel_to_target("3"), SPECIAL_MULTI_USER_TARGET)); - assert_se(streq_ptr(runlevel_to_target("rd.rescue"), NULL)); - - in_initrd_force(true); - assert_se(streq_ptr(runlevel_to_target(NULL), NULL)); - assert_se(streq_ptr(runlevel_to_target("unknown-runlevel"), NULL)); - assert_se(streq_ptr(runlevel_to_target("rd.unknown-runlevel"), NULL)); - assert_se(streq_ptr(runlevel_to_target("3"), NULL)); - assert_se(streq_ptr(runlevel_to_target("rd.rescue"), SPECIAL_RESCUE_TARGET)); -} - int main(void) { log_parse_environment(); log_open(); @@ -244,7 +226,6 @@ int main(void) { test_proc_cmdline_get_key(); test_proc_cmdline_get_bool(); test_proc_cmdline_get_key_many(); - test_runlevel_to_target(); return 0; } diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c index 8bc5bf6038..f0186b078f 100644 --- a/src/test/test-unit-file.c +++ b/src/test/test-unit-file.c @@ -2,6 +2,7 @@ #include "path-lookup.h" #include "set.h" +#include "special.h" #include "strv.h" #include "tests.h" #include "unit-file.h" @@ -75,11 +76,30 @@ static void test_unit_file_build_name_map(char **ids) { } } +static void test_runlevel_to_target(void) { + log_info("/* %s */", __func__); + + in_initrd_force(false); + assert_se(streq_ptr(runlevel_to_target(NULL), NULL)); + assert_se(streq_ptr(runlevel_to_target("unknown-runlevel"), NULL)); + assert_se(streq_ptr(runlevel_to_target("rd.unknown-runlevel"), NULL)); + assert_se(streq_ptr(runlevel_to_target("3"), SPECIAL_MULTI_USER_TARGET)); + assert_se(streq_ptr(runlevel_to_target("rd.rescue"), NULL)); + + in_initrd_force(true); + assert_se(streq_ptr(runlevel_to_target(NULL), NULL)); + assert_se(streq_ptr(runlevel_to_target("unknown-runlevel"), NULL)); + assert_se(streq_ptr(runlevel_to_target("rd.unknown-runlevel"), NULL)); + assert_se(streq_ptr(runlevel_to_target("3"), NULL)); + assert_se(streq_ptr(runlevel_to_target("rd.rescue"), SPECIAL_RESCUE_TARGET)); +} + int main(int argc, char **argv) { test_setup_logging(LOG_DEBUG); test_unit_validate_alias_symlink_and_warn(); test_unit_file_build_name_map(strv_skip(argv, 1)); + test_runlevel_to_target(); return 0; } From 2bfa8466c079b81b016f8a4ab7958eb085eb5a7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Aug 2019 14:34:51 +0200 Subject: [PATCH 05/10] util-lib: move shall_restore_state() to shared/reboot-util It's just a small function, but it is higher-level functionality. I don't see a good place for it, reboot-util.[ch] seems least bad --- src/backlight/backlight.c | 2 +- src/basic/proc-cmdline.c | 11 ----------- src/basic/proc-cmdline.h | 2 -- src/rfkill/rfkill.c | 4 ++-- src/shared/reboot-util.c | 12 ++++++++++++ src/shared/reboot-util.h | 2 ++ 6 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c index dfd6805398..0484414290 100644 --- a/src/backlight/backlight.c +++ b/src/backlight/backlight.c @@ -13,7 +13,7 @@ #include "main-func.h" #include "mkdir.h" #include "parse-util.h" -#include "proc-cmdline.h" +#include "reboot-util.h" #include "string-util.h" #include "strv.h" #include "util.h" diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index 5d29774e09..23cda75708 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -306,14 +306,3 @@ int proc_cmdline_get_key_many_internal(ProcCmdlineFlags flags, ...) { return ret; } - -int shall_restore_state(void) { - bool ret; - int r; - - r = proc_cmdline_get_bool("systemd.restore_state", &ret); - if (r < 0) - return r; - - return r > 0 ? ret : true; -} diff --git a/src/basic/proc-cmdline.h b/src/basic/proc-cmdline.h index 429e7ca4a4..4115fdbc99 100644 --- a/src/basic/proc-cmdline.h +++ b/src/basic/proc-cmdline.h @@ -27,8 +27,6 @@ int proc_cmdline_get_key_many_internal(ProcCmdlineFlags flags, ...); char *proc_cmdline_key_startswith(const char *s, const char *prefix); bool proc_cmdline_key_streq(const char *x, const char *y); -int shall_restore_state(void); - /* A little helper call, to be used in proc_cmdline_parse_t callbacks */ static inline bool proc_cmdline_value_missing(const char *key, const char *value) { if (!value) { diff --git a/src/rfkill/rfkill.c b/src/rfkill/rfkill.c index f9d309c909..501982819a 100644 --- a/src/rfkill/rfkill.c +++ b/src/rfkill/rfkill.c @@ -16,15 +16,15 @@ #include "fd-util.h" #include "fileio.h" #include "io-util.h" +#include "list.h" #include "main-func.h" #include "mkdir.h" #include "parse-util.h" -#include "proc-cmdline.h" +#include "reboot-util.h" #include "string-table.h" #include "string-util.h" #include "udev-util.h" #include "util.h" -#include "list.h" /* Note that any write is delayed until exit and the rfkill state will not be * stored for rfkill indices that disappear after a change. */ diff --git a/src/shared/reboot-util.c b/src/shared/reboot-util.c index 08569e8bf3..5d76299179 100644 --- a/src/shared/reboot-util.c +++ b/src/shared/reboot-util.c @@ -6,6 +6,7 @@ #include "alloc-util.h" #include "fileio.h" #include "log.h" +#include "proc-cmdline.h" #include "raw-reboot.h" #include "reboot-util.h" #include "string-util.h" @@ -96,3 +97,14 @@ int reboot_with_parameter(RebootFlags flags) { return log_full_errno(flags & REBOOT_LOG ? LOG_ERR : LOG_DEBUG, errno, "Failed to reboot: %m"); } + +int shall_restore_state(void) { + bool ret; + int r; + + r = proc_cmdline_get_bool("systemd.restore_state", &ret); + if (r < 0) + return r; + + return r > 0 ? ret : true; +} diff --git a/src/shared/reboot-util.h b/src/shared/reboot-util.h index 7bddc91ea6..5aeb34821f 100644 --- a/src/shared/reboot-util.h +++ b/src/shared/reboot-util.h @@ -11,3 +11,5 @@ typedef enum RebootFlags { int read_reboot_parameter(char **parameter); int reboot_with_parameter(RebootFlags flags); + +int shall_restore_state(void); From 0bb2f0f10e627d33cf3872b2ec35c0454a517ca1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Aug 2019 16:28:29 +0200 Subject: [PATCH 06/10] util-lib: split shared/efivars into basic/efivars and shared/efi-loader I want to use efivars.[ch] in proc-cmdline.c, but most of the efivars stuff is not needed in basic/. Move the file from shared/ to basic/, but then move back most of the higher-level functions to the new shared/efi-loader.c file. --- src/basic/efivars.c | 225 ++++++++++++++++++++ src/basic/efivars.h | 52 +++++ src/basic/meson.build | 2 + src/boot/bless-boot-generator.c | 2 +- src/boot/bless-boot.c | 1 + src/boot/bootctl.c | 1 + src/core/mount-setup.c | 3 +- src/gpt-auto-generator/gpt-auto-generator.c | 2 +- src/login/logind-dbus.c | 1 + src/shared/boot-timestamps.c | 2 +- src/shared/bootspec.c | 1 + src/shared/condition.c | 2 +- src/shared/{efivars.c => efi-loader.c} | 212 +----------------- src/shared/{efivars.h => efi-loader.h} | 44 +--- src/shared/meson.build | 4 +- src/test/test-boot-timestamps.c | 2 +- src/test/test-condition.c | 2 +- src/udev/udev-builtin-blkid.c | 2 +- 18 files changed, 297 insertions(+), 263 deletions(-) create mode 100644 src/basic/efivars.c create mode 100644 src/basic/efivars.h rename src/shared/{efivars.c => efi-loader.c} (79%) rename src/shared/{efivars.h => efi-loader.h} (61%) diff --git a/src/basic/efivars.c b/src/basic/efivars.c new file mode 100644 index 0000000000..5264ab1b50 --- /dev/null +++ b/src/basic/efivars.c @@ -0,0 +1,225 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "sd-id128.h" + +#include "alloc-util.h" +#include "chattr-util.h" +#include "efivars.h" +#include "fd-util.h" +#include "io-util.h" +#include "macro.h" +#include "stdio-util.h" +#include "strv.h" +#include "time-util.h" +#include "utf8.h" + +#if ENABLE_EFI + +char* efi_variable_path(sd_id128_t vendor, const char *name) { + char *p; + + if (asprintf(&p, + "/sys/firmware/efi/efivars/%s-" SD_ID128_UUID_FORMAT_STR, + name, SD_ID128_FORMAT_VAL(vendor)) < 0) + return NULL; + + return p; +} + +int efi_get_variable( + sd_id128_t vendor, + const char *name, + uint32_t *ret_attribute, + void **ret_value, + size_t *ret_size) { + + _cleanup_close_ int fd = -1; + _cleanup_free_ char *p = NULL; + _cleanup_free_ void *buf = NULL; + struct stat st; + uint32_t a; + ssize_t n; + + assert(name); + + p = efi_variable_path(vendor, name); + if (!p) + return -ENOMEM; + + if (!ret_value && !ret_size && !ret_attribute) { + /* If caller is not interested in anything, just check if the variable exists and is readable + * to us. */ + if (access(p, R_OK) < 0) + return -errno; + + return 0; + } + + fd = open(p, O_RDONLY|O_NOCTTY|O_CLOEXEC); + if (fd < 0) + return -errno; + + if (fstat(fd, &st) < 0) + return -errno; + if (st.st_size < 4) + return -ENODATA; + if (st.st_size > 4*1024*1024 + 4) + return -E2BIG; + + if (ret_value || ret_attribute) { + n = read(fd, &a, sizeof(a)); + if (n < 0) + return -errno; + if (n != sizeof(a)) + return -EIO; + } + + if (ret_value) { + buf = malloc(st.st_size - 4 + 2); + if (!buf) + return -ENOMEM; + + n = read(fd, buf, (size_t) st.st_size - 4); + if (n < 0) + return -errno; + if (n != st.st_size - 4) + return -EIO; + + /* Always NUL terminate (2 bytes, to protect UTF-16) */ + ((char*) buf)[st.st_size - 4] = 0; + ((char*) buf)[st.st_size - 4 + 1] = 0; + } + + /* Note that efivarfs interestingly doesn't require ftruncate() to update an existing EFI variable + * with a smaller value. */ + + if (ret_attribute) + *ret_attribute = a; + + if (ret_value) + *ret_value = TAKE_PTR(buf); + + if (ret_size) + *ret_size = (size_t) st.st_size - 4; + + return 0; +} + +int efi_get_variable_string(sd_id128_t vendor, const char *name, char **p) { + _cleanup_free_ void *s = NULL; + size_t ss = 0; + int r; + char *x; + + r = efi_get_variable(vendor, name, NULL, &s, &ss); + if (r < 0) + return r; + + x = utf16_to_utf8(s, ss); + if (!x) + return -ENOMEM; + + *p = x; + return 0; +} + +int efi_set_variable( + sd_id128_t vendor, + const char *name, + const void *value, + size_t size) { + + struct var { + uint32_t attr; + char buf[]; + } _packed_ * _cleanup_free_ buf = NULL; + _cleanup_free_ char *p = NULL; + _cleanup_close_ int fd = -1; + bool saved_flags_valid = false; + unsigned saved_flags; + int r; + + assert(name); + assert(value || size == 0); + + p = efi_variable_path(vendor, name); + if (!p) + return -ENOMEM; + + /* Newer efivarfs protects variables that are not in a whitelist with FS_IMMUTABLE_FL by default, to protect + * them for accidental removal and modification. We are not changing these variables accidentally however, + * hence let's unset the bit first. */ + + r = chattr_path(p, 0, FS_IMMUTABLE_FL, &saved_flags); + if (r < 0 && r != -ENOENT) + log_debug_errno(r, "Failed to drop FS_IMMUTABLE_FL flag from '%s', ignoring: %m", p); + + saved_flags_valid = r >= 0; + + if (size == 0) { + if (unlink(p) < 0) { + r = -errno; + goto finish; + } + + return 0; + } + + fd = open(p, O_WRONLY|O_CREAT|O_NOCTTY|O_CLOEXEC, 0644); + if (fd < 0) { + r = -errno; + goto finish; + } + + buf = malloc(sizeof(uint32_t) + size); + if (!buf) { + r = -ENOMEM; + goto finish; + } + + buf->attr = EFI_VARIABLE_NON_VOLATILE|EFI_VARIABLE_BOOTSERVICE_ACCESS|EFI_VARIABLE_RUNTIME_ACCESS; + memcpy(buf->buf, value, size); + + r = loop_write(fd, buf, sizeof(uint32_t) + size, false); + if (r < 0) + goto finish; + + r = 0; + +finish: + if (saved_flags_valid) { + int q; + + /* Restore the original flags field, just in case */ + if (fd < 0) + q = chattr_path(p, saved_flags, FS_IMMUTABLE_FL, NULL); + else + q = chattr_fd(fd, saved_flags, FS_IMMUTABLE_FL, NULL); + if (q < 0) + log_debug_errno(q, "Failed to restore FS_IMMUTABLE_FL on '%s', ignoring: %m", p); + } + + return r; +} + +int efi_set_variable_string(sd_id128_t vendor, const char *name, const char *v) { + _cleanup_free_ char16_t *u16 = NULL; + + u16 = utf8_to_utf16(v, strlen(v)); + if (!u16) + return -ENOMEM; + + return efi_set_variable(vendor, name, u16, (char16_strlen(u16) + 1) * sizeof(char16_t)); +} + +#endif diff --git a/src/basic/efivars.h b/src/basic/efivars.h new file mode 100644 index 0000000000..e8cb5b8a9a --- /dev/null +++ b/src/basic/efivars.h @@ -0,0 +1,52 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ +#pragma once + +#if !ENABLE_EFI +# include +#endif +#include +#include +#include + +#include "sd-id128.h" + +#include "efi/loader-features.h" +#include "time-util.h" + +#define EFI_VENDOR_LOADER SD_ID128_MAKE(4a,67,b0,82,0a,4c,41,cf,b6,c7,44,0b,29,bb,8c,4f) +#define EFI_VENDOR_GLOBAL SD_ID128_MAKE(8b,e4,df,61,93,ca,11,d2,aa,0d,00,e0,98,03,2b,8c) +#define EFI_VARIABLE_NON_VOLATILE 0x0000000000000001 +#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0000000000000002 +#define EFI_VARIABLE_RUNTIME_ACCESS 0x0000000000000004 + +#if ENABLE_EFI + +char* efi_variable_path(sd_id128_t vendor, const char *name); +int efi_get_variable(sd_id128_t vendor, const char *name, uint32_t *attribute, void **value, size_t *size); +int efi_get_variable_string(sd_id128_t vendor, const char *name, char **p); +int efi_set_variable(sd_id128_t vendor, const char *name, const void *value, size_t size); +int efi_set_variable_string(sd_id128_t vendor, const char *name, const char *p); + +#else + +static inline char* efi_variable_path(sd_id128_t vendor, const char *name) { + return NULL; +} + +static inline int efi_get_variable(sd_id128_t vendor, const char *name, uint32_t *attribute, void **value, size_t *size) { + return -EOPNOTSUPP; +} + +static inline int efi_get_variable_string(sd_id128_t vendor, const char *name, char **p) { + return -EOPNOTSUPP; +} + +static inline int efi_set_variable(sd_id128_t vendor, const char *name, const void *value, size_t size) { + return -EOPNOTSUPP; +} + +static inline int efi_set_variable_string(sd_id128_t vendor, const char *name, const char *p) { + return -EOPNOTSUPP; +} + +#endif diff --git a/src/basic/meson.build b/src/basic/meson.build index d6caf28f14..43ab1849f9 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -39,6 +39,8 @@ basic_sources = files(''' device-nodes.h dirent-util.c dirent-util.h + efivars.c + efivars.h env-file.c env-file.h env-util.c diff --git a/src/boot/bless-boot-generator.c b/src/boot/bless-boot-generator.c index e28cccd761..c59d8aed90 100644 --- a/src/boot/bless-boot-generator.c +++ b/src/boot/bless-boot-generator.c @@ -4,7 +4,7 @@ #include #include -#include "efivars.h" +#include "efi-loader.h" #include "generator.h" #include "log.h" #include "mkdir.h" diff --git a/src/boot/bless-boot.c b/src/boot/bless-boot.c index f2d033fc40..4747e7fb4f 100644 --- a/src/boot/bless-boot.c +++ b/src/boot/bless-boot.c @@ -5,6 +5,7 @@ #include "alloc-util.h" #include "bootspec.h" +#include "efi-loader.h" #include "efivars.h" #include "fd-util.h" #include "fs-util.h" diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index ddc267401f..097c796a4e 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -24,6 +24,7 @@ #include "bootspec.h" #include "copy.h" #include "dirent-util.h" +#include "efi-loader.h" #include "efivars.h" #include "env-util.h" #include "escape.h" diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c index 790f1e234e..9f2e7ff985 100644 --- a/src/core/mount-setup.c +++ b/src/core/mount-setup.c @@ -13,7 +13,8 @@ #include "conf-files.h" #include "cgroup-setup.h" #include "dev-setup.h" -#include "efivars.h" +#include "dirent-util.h" +#include "efi-loader.h" #include "fd-util.h" #include "fileio.h" #include "fs-util.h" diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index 49149b59be..5002eb9d74 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -15,7 +15,7 @@ #include "device-util.h" #include "dirent-util.h" #include "dissect-image.h" -#include "efivars.h" +#include "efi-loader.h" #include "fd-util.h" #include "fileio.h" #include "fs-util.h" diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 30b9a66334..1d76ec6885 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -20,6 +20,7 @@ #include "device-util.h" #include "dirent-util.h" #include "efivars.h" +#include "efi-loader.h" #include "env-util.h" #include "escape.h" #include "fd-util.h" diff --git a/src/shared/boot-timestamps.c b/src/shared/boot-timestamps.c index bcbb86d1b1..4ce146033d 100644 --- a/src/shared/boot-timestamps.c +++ b/src/shared/boot-timestamps.c @@ -2,7 +2,7 @@ #include "acpi-fpdt.h" #include "boot-timestamps.h" -#include "efivars.h" +#include "efi-loader.h" #include "macro.h" #include "time-util.h" diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 3b4c99e9ee..5221862a68 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -15,6 +15,7 @@ #include "device-nodes.h" #include "dirent-util.h" #include "efivars.h" +#include "efi-loader.h" #include "env-file.h" #include "env-util.h" #include "fd-util.h" diff --git a/src/shared/condition.c b/src/shared/condition.c index e5e6c6cc13..5a5d35bcc3 100644 --- a/src/shared/condition.c +++ b/src/shared/condition.c @@ -22,7 +22,7 @@ #include "cgroup-util.h" #include "condition.h" #include "cpu-set-util.h" -#include "efivars.h" +#include "efi-loader.h" #include "env-file.h" #include "extract-word.h" #include "fd-util.h" diff --git a/src/shared/efivars.c b/src/shared/efi-loader.c similarity index 79% rename from src/shared/efivars.c rename to src/shared/efi-loader.c index 3597ddf4a7..46e187000f 100644 --- a/src/shared/efivars.c +++ b/src/shared/efi-loader.c @@ -1,30 +1,18 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ -#include -#include -#include -#include -#include -#include #include -#include -#include #include -#include "sd-id128.h" - #include "alloc-util.h" -#include "chattr-util.h" #include "dirent-util.h" +#include "efi-loader.h" #include "efivars.h" #include "fd-util.h" #include "io-util.h" -#include "macro.h" #include "parse-util.h" #include "sort-util.h" #include "stdio-util.h" -#include "strv.h" -#include "time-util.h" +#include "string-util.h" #include "utf8.h" #include "virt.h" @@ -193,202 +181,6 @@ int efi_set_reboot_to_firmware(bool value) { return 0; } -char* efi_variable_path(sd_id128_t vendor, const char *name) { - char *p; - - if (asprintf(&p, - "/sys/firmware/efi/efivars/%s-" SD_ID128_UUID_FORMAT_STR, - name, SD_ID128_FORMAT_VAL(vendor)) < 0) - return NULL; - - return p; -} - -int efi_get_variable( - sd_id128_t vendor, - const char *name, - uint32_t *ret_attribute, - void **ret_value, - size_t *ret_size) { - - _cleanup_close_ int fd = -1; - _cleanup_free_ char *p = NULL; - _cleanup_free_ void *buf = NULL; - struct stat st; - uint32_t a; - ssize_t n; - - assert(name); - - p = efi_variable_path(vendor, name); - if (!p) - return -ENOMEM; - - if (!ret_value && !ret_size && !ret_attribute) { - /* If caller is not interested in anything, just check if the variable exists and is readable - * to us. */ - if (access(p, R_OK) < 0) - return -errno; - - return 0; - } - - fd = open(p, O_RDONLY|O_NOCTTY|O_CLOEXEC); - if (fd < 0) - return -errno; - - if (fstat(fd, &st) < 0) - return -errno; - if (st.st_size < 4) - return -ENODATA; - if (st.st_size > 4*1024*1024 + 4) - return -E2BIG; - - if (ret_value || ret_attribute) { - n = read(fd, &a, sizeof(a)); - if (n < 0) - return -errno; - if (n != sizeof(a)) - return -EIO; - } - - if (ret_value) { - buf = malloc(st.st_size - 4 + 2); - if (!buf) - return -ENOMEM; - - n = read(fd, buf, (size_t) st.st_size - 4); - if (n < 0) - return -errno; - if (n != st.st_size - 4) - return -EIO; - - /* Always NUL terminate (2 bytes, to protect UTF-16) */ - ((char*) buf)[st.st_size - 4] = 0; - ((char*) buf)[st.st_size - 4 + 1] = 0; - } - - /* Note that efivarfs interestingly doesn't require ftruncate() to update an existing EFI variable - * with a smaller value. */ - - if (ret_attribute) - *ret_attribute = a; - - if (ret_value) - *ret_value = TAKE_PTR(buf); - - if (ret_size) - *ret_size = (size_t) st.st_size - 4; - - return 0; -} - -int efi_get_variable_string(sd_id128_t vendor, const char *name, char **p) { - _cleanup_free_ void *s = NULL; - size_t ss = 0; - int r; - char *x; - - r = efi_get_variable(vendor, name, NULL, &s, &ss); - if (r < 0) - return r; - - x = utf16_to_utf8(s, ss); - if (!x) - return -ENOMEM; - - *p = x; - return 0; -} - -int efi_set_variable( - sd_id128_t vendor, - const char *name, - const void *value, - size_t size) { - - struct var { - uint32_t attr; - char buf[]; - } _packed_ * _cleanup_free_ buf = NULL; - _cleanup_free_ char *p = NULL; - _cleanup_close_ int fd = -1; - bool saved_flags_valid = false; - unsigned saved_flags; - int r; - - assert(name); - assert(value || size == 0); - - p = efi_variable_path(vendor, name); - if (!p) - return -ENOMEM; - - /* Newer efivarfs protects variables that are not in a whitelist with FS_IMMUTABLE_FL by default, to protect - * them for accidental removal and modification. We are not changing these variables accidentally however, - * hence let's unset the bit first. */ - - r = chattr_path(p, 0, FS_IMMUTABLE_FL, &saved_flags); - if (r < 0 && r != -ENOENT) - log_debug_errno(r, "Failed to drop FS_IMMUTABLE_FL flag from '%s', ignoring: %m", p); - - saved_flags_valid = r >= 0; - - if (size == 0) { - if (unlink(p) < 0) { - r = -errno; - goto finish; - } - - return 0; - } - - fd = open(p, O_WRONLY|O_CREAT|O_NOCTTY|O_CLOEXEC, 0644); - if (fd < 0) { - r = -errno; - goto finish; - } - - buf = malloc(sizeof(uint32_t) + size); - if (!buf) { - r = -ENOMEM; - goto finish; - } - - buf->attr = EFI_VARIABLE_NON_VOLATILE|EFI_VARIABLE_BOOTSERVICE_ACCESS|EFI_VARIABLE_RUNTIME_ACCESS; - memcpy(buf->buf, value, size); - - r = loop_write(fd, buf, sizeof(uint32_t) + size, false); - if (r < 0) - goto finish; - - r = 0; - -finish: - if (saved_flags_valid) { - int q; - - /* Restore the original flags field, just in case */ - if (fd < 0) - q = chattr_path(p, saved_flags, FS_IMMUTABLE_FL, NULL); - else - q = chattr_fd(fd, saved_flags, FS_IMMUTABLE_FL, NULL); - if (q < 0) - log_debug_errno(q, "Failed to restore FS_IMMUTABLE_FL on '%s', ignoring: %m", p); - } - - return r; -} - -int efi_set_variable_string(sd_id128_t vendor, const char *name, const char *v) { - _cleanup_free_ char16_t *u16 = NULL; - - u16 = utf8_to_utf16(v, strlen(v)); - if (!u16) - return -ENOMEM; - - return efi_set_variable(vendor, name, u16, (char16_strlen(u16) + 1) * sizeof(char16_t)); -} static ssize_t utf16_size(const uint16_t *s, size_t buf_len_bytes) { size_t l = 0; diff --git a/src/shared/efivars.h b/src/shared/efi-loader.h similarity index 61% rename from src/shared/efivars.h rename to src/shared/efi-loader.h index fad129794d..7d41fbb359 100644 --- a/src/shared/efivars.h +++ b/src/shared/efi-loader.h @@ -1,23 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #pragma once -#if ! ENABLE_EFI -#include -#endif -#include -#include -#include - -#include "sd-id128.h" - -#include "efi/loader-features.h" -#include "time-util.h" - -#define EFI_VENDOR_LOADER SD_ID128_MAKE(4a,67,b0,82,0a,4c,41,cf,b6,c7,44,0b,29,bb,8c,4f) -#define EFI_VENDOR_GLOBAL SD_ID128_MAKE(8b,e4,df,61,93,ca,11,d2,aa,0d,00,e0,98,03,2b,8c) -#define EFI_VARIABLE_NON_VOLATILE 0x0000000000000001 -#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0000000000000002 -#define EFI_VARIABLE_RUNTIME_ACCESS 0x0000000000000004 +#include "efivars.h" #if ENABLE_EFI @@ -28,12 +12,6 @@ int efi_reboot_to_firmware_supported(void); int efi_get_reboot_to_firmware(void); int efi_set_reboot_to_firmware(bool value); -char* efi_variable_path(sd_id128_t vendor, const char *name); -int efi_get_variable(sd_id128_t vendor, const char *name, uint32_t *attribute, void **value, size_t *size); -int efi_get_variable_string(sd_id128_t vendor, const char *name, char **p); -int efi_set_variable(sd_id128_t vendor, const char *name, const void *value, size_t size); -int efi_set_variable_string(sd_id128_t vendor, const char *name, const char *p); - int efi_get_boot_option(uint16_t nr, char **title, sd_id128_t *part_uuid, char **path, bool *active); int efi_add_boot_option(uint16_t id, const char *title, uint32_t part, uint64_t pstart, uint64_t psize, sd_id128_t part_uuid, const char *path); int efi_remove_boot_option(uint16_t id); @@ -74,26 +52,6 @@ static inline int efi_set_reboot_to_firmware(bool value) { return -EOPNOTSUPP; } -static inline char* efi_variable_path(sd_id128_t vendor, const char *name) { - return NULL; -} - -static inline int efi_get_variable(sd_id128_t vendor, const char *name, uint32_t *attribute, void **value, size_t *size) { - return -EOPNOTSUPP; -} - -static inline int efi_get_variable_string(sd_id128_t vendor, const char *name, char **p) { - return -EOPNOTSUPP; -} - -static inline int efi_set_variable(sd_id128_t vendor, const char *name, const void *value, size_t size) { - return -EOPNOTSUPP; -} - -static inline int efi_set_variable_string(sd_id128_t vendor, const char *name, const char *p) { - return -EOPNOTSUPP; -} - static inline int efi_get_boot_option(uint16_t nr, char **title, sd_id128_t *part_uuid, char **path, bool *active) { return -EOPNOTSUPP; } diff --git a/src/shared/meson.build b/src/shared/meson.build index 63a3f88d50..40412de433 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -60,8 +60,8 @@ shared_sources = files(''' dns-domain.h dropin.c dropin.h - efivars.c - efivars.h + efi-loader.c + efi-loader.h enable-mempool.c env-file-label.c env-file-label.h diff --git a/src/test/test-boot-timestamps.c b/src/test/test-boot-timestamps.c index 79b8dd49a7..3c7f7a98cf 100644 --- a/src/test/test-boot-timestamps.c +++ b/src/test/test-boot-timestamps.c @@ -5,7 +5,7 @@ #include "acpi-fpdt.h" #include "boot-timestamps.h" -#include "efivars.h" +#include "efi-loader.h" #include "log.h" #include "tests.h" #include "util.h" diff --git a/src/test/test-condition.c b/src/test/test-condition.c index 9a0a4cfee2..fce9232dcf 100644 --- a/src/test/test-condition.c +++ b/src/test/test-condition.c @@ -14,7 +14,7 @@ #include "cgroup-util.h" #include "condition.h" #include "cpu-set-util.h" -#include "efivars.h" +#include "efi-loader.h" #include "hostname-util.h" #include "id128-util.h" #include "ima-util.h" diff --git a/src/udev/udev-builtin-blkid.c b/src/udev/udev-builtin-blkid.c index 7ef75e6f91..c6b0208b4b 100644 --- a/src/udev/udev-builtin-blkid.c +++ b/src/udev/udev-builtin-blkid.c @@ -19,7 +19,7 @@ #include "alloc-util.h" #include "blkid-util.h" #include "device-util.h" -#include "efivars.h" +#include "efi-loader.h" #include "errno-util.h" #include "fd-util.h" #include "gpt.h" From e825ea2f4c7e910b00cc8fd8bb6b93c777a12f78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 20 Aug 2019 12:11:24 +0200 Subject: [PATCH 07/10] test-proc-cmdline: use test_setup_logging() --- src/test/test-proc-cmdline.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/test-proc-cmdline.c b/src/test/test-proc-cmdline.c index 884fb36010..1a00642f86 100644 --- a/src/test/test-proc-cmdline.c +++ b/src/test/test-proc-cmdline.c @@ -7,6 +7,7 @@ #include "proc-cmdline.h" #include "special.h" #include "string-util.h" +#include "tests.h" #include "util.h" static int obj; @@ -213,8 +214,7 @@ static void test_proc_cmdline_key_startswith(void) { } int main(void) { - log_parse_environment(); - log_open(); + test_setup_logging(LOG_INFO); test_proc_cmdline_parse(); test_proc_cmdline_override(); From 53aa0d02add93d8c0afa2772609372a2040c162a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Aug 2019 15:33:35 +0200 Subject: [PATCH 08/10] Add support for SystemdOptions EFI var to augment /proc/cmdline In various circumstances, overriding the kernel commandline can be inconvenient. People have different bootloaders, and e.g. the grub config can be pretty scary. grubby helps, but it isn't always available. This option adds an alternative mechanism that can quite convenient on EFI systems. cmdline settings have higher priority, because they can be (usually) changed on the bootloader prompt. $SYSTEMD_EFI_OPTIONS can be used to override, same as $SYSTEMD_PROC_CMDLINE. --- docs/ENVIRONMENT.md | 11 +++-- src/basic/efivars.c | 25 +++++++++++ src/basic/efivars.h | 11 ++++- src/basic/proc-cmdline.c | 82 +++++++++++++++++++++++++----------- src/test/test-proc-cmdline.c | 34 ++++++++++++++- 5 files changed, 132 insertions(+), 31 deletions(-) diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md index 838cc7e4cf..f9025247d8 100644 --- a/docs/ENVIRONMENT.md +++ b/docs/ENVIRONMENT.md @@ -36,10 +36,13 @@ All tools: * `$SD_EVENT_PROFILE_DELAYS=1` — if set, the sd-event event loop implementation will print latency information at runtime. -* `$SYSTEMD_PROC_CMDLINE` — if set, may contain a string that is used as kernel - command line instead of the actual one readable from /proc/cmdline. This is - useful for debugging, in order to test generators and other code against - specific kernel command lines. +* `$SYSTEMD_PROC_CMDLINE` — if set, the contents are used as the kernel command + line instead of the actual one in /proc/cmdline. This is useful for + debugging, in order to test generators and other code against specific kernel + command lines. + +* `$SYSTEMD_EFI_OPTIONS` — if set, used instead of the string in SystemdOptions + EFI variable. Analogous to `$SYSTEMD_PROC_CMDLINE`. * `$SYSTEMD_IN_INITRD` — takes a boolean. If set, overrides initrd detection. This is useful for debugging and testing initrd-only programs in the main diff --git a/src/basic/efivars.c b/src/basic/efivars.c index 5264ab1b50..53875de5a0 100644 --- a/src/basic/efivars.c +++ b/src/basic/efivars.c @@ -222,4 +222,29 @@ int efi_set_variable_string(sd_id128_t vendor, const char *name, const char *v) return efi_set_variable(vendor, name, u16, (char16_strlen(u16) + 1) * sizeof(char16_t)); } +int efi_systemd_options_variable(char **line) { + const char *e; + int r; + + assert(line); + + /* For testing purposes it is sometimes useful to be able to override this */ + e = secure_getenv("SYSTEMD_EFI_OPTIONS"); + if (e) { + char *m; + + m = strdup(e); + if (!m) + return -ENOMEM; + + *line = m; + return 0; + } + + r = efi_get_variable_string(EFI_VENDOR_SYSTEMD, "SystemdOptions", line); + if (r == -ENOENT) + return -ENODATA; + + return r; +} #endif diff --git a/src/basic/efivars.h b/src/basic/efivars.h index e8cb5b8a9a..22edbed985 100644 --- a/src/basic/efivars.h +++ b/src/basic/efivars.h @@ -13,8 +13,9 @@ #include "efi/loader-features.h" #include "time-util.h" -#define EFI_VENDOR_LOADER SD_ID128_MAKE(4a,67,b0,82,0a,4c,41,cf,b6,c7,44,0b,29,bb,8c,4f) -#define EFI_VENDOR_GLOBAL SD_ID128_MAKE(8b,e4,df,61,93,ca,11,d2,aa,0d,00,e0,98,03,2b,8c) +#define EFI_VENDOR_LOADER SD_ID128_MAKE(4a,67,b0,82,0a,4c,41,cf,b6,c7,44,0b,29,bb,8c,4f) +#define EFI_VENDOR_GLOBAL SD_ID128_MAKE(8b,e4,df,61,93,ca,11,d2,aa,0d,00,e0,98,03,2b,8c) +#define EFI_VENDOR_SYSTEMD SD_ID128_MAKE(8c,f2,64,4b,4b,0b,42,8f,93,87,6d,87,60,50,dc,67) #define EFI_VARIABLE_NON_VOLATILE 0x0000000000000001 #define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0000000000000002 #define EFI_VARIABLE_RUNTIME_ACCESS 0x0000000000000004 @@ -27,6 +28,8 @@ int efi_get_variable_string(sd_id128_t vendor, const char *name, char **p); int efi_set_variable(sd_id128_t vendor, const char *name, const void *value, size_t size); int efi_set_variable_string(sd_id128_t vendor, const char *name, const char *p); +int efi_systemd_options_variable(char **line); + #else static inline char* efi_variable_path(sd_id128_t vendor, const char *name) { @@ -49,4 +52,8 @@ static inline int efi_set_variable_string(sd_id128_t vendor, const char *name, c return -EOPNOTSUPP; } +static inline int efi_systemd_options_variable(char **line) { + return -ENODATA; +} + #endif diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index 23cda75708..44d1e9aec4 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -5,6 +5,7 @@ #include #include "alloc-util.h" +#include "efivars.h" #include "extract-word.h" #include "fileio.h" #include "macro.h" @@ -117,6 +118,17 @@ int proc_cmdline_parse(proc_cmdline_parse_t parse_item, void *data, ProcCmdlineF assert(parse_item); + /* We parse the EFI variable first, because later settings have higher priority. */ + + r = efi_systemd_options_variable(&line); + if (r < 0 && r != -ENODATA) + log_debug_errno(r, "Failed to get SystemdOptions EFI variable, ignoring: %m"); + + r = proc_cmdline_parse_given(line, parse_item, data, flags); + if (r < 0) + return r; + + line = mfree(line); r = proc_cmdline(&line); if (r < 0) return r; @@ -156,34 +168,14 @@ bool proc_cmdline_key_streq(const char *x, const char *y) { return true; } -int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_value) { - _cleanup_free_ char *line = NULL, *ret = NULL; +static int cmdline_get_key(const char *line, const char *key, ProcCmdlineFlags flags, char **ret_value) { + _cleanup_free_ char *ret = NULL; bool found = false; const char *p; int r; - /* Looks for a specific key on the kernel command line. Supports three modes: - * - * a) The "ret_value" parameter is used. In this case a parameter beginning with the "key" string followed by - * "=" is searched for, and the value following it is returned in "ret_value". - * - * b) as above, but the PROC_CMDLINE_VALUE_OPTIONAL flag is set. In this case if the key is found as a separate - * word (i.e. not followed by "=" but instead by whitespace or the end of the command line), then this is - * also accepted, and "value" is returned as NULL. - * - * c) The "ret_value" parameter is NULL. In this case a search for the exact "key" parameter is performed. - * - * In all three cases, > 0 is returned if the key is found, 0 if not. */ - - if (isempty(key)) - return -EINVAL; - - if (FLAGS_SET(flags, PROC_CMDLINE_VALUE_OPTIONAL) && !ret_value) - return -EINVAL; - - r = proc_cmdline(&line); - if (r < 0) - return r; + assert(line); + assert(key); p = line; for (;;) { @@ -226,6 +218,48 @@ int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_val return found; } +int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_value) { + _cleanup_free_ char *line = NULL; + int r; + + /* Looks for a specific key on the kernel command line and (with lower priority) the EFI variable. + * Supports three modes: + * + * a) The "ret_value" parameter is used. In this case a parameter beginning with the "key" string followed by + * "=" is searched for, and the value following it is returned in "ret_value". + * + * b) as above, but the PROC_CMDLINE_VALUE_OPTIONAL flag is set. In this case if the key is found as a separate + * word (i.e. not followed by "=" but instead by whitespace or the end of the command line), then this is + * also accepted, and "value" is returned as NULL. + * + * c) The "ret_value" parameter is NULL. In this case a search for the exact "key" parameter is performed. + * + * In all three cases, > 0 is returned if the key is found, 0 if not. */ + + if (isempty(key)) + return -EINVAL; + + if (FLAGS_SET(flags, PROC_CMDLINE_VALUE_OPTIONAL) && !ret_value) + return -EINVAL; + + r = proc_cmdline(&line); + if (r < 0) + return r; + + r = cmdline_get_key(line, key, flags, ret_value); + if (r != 0) /* Either error or true if found. */ + return r; + + line = mfree(line); + r = efi_systemd_options_variable(&line); + if (r == -ENODATA) + return false; /* Not found */ + if (r < 0) + return r; + + return cmdline_get_key(line, key, flags, ret_value); +} + int proc_cmdline_get_bool(const char *key, bool *ret) { _cleanup_free_ char *v = NULL; int r; diff --git a/src/test/test-proc-cmdline.c b/src/test/test-proc-cmdline.c index 1a00642f86..3231e4a3e6 100644 --- a/src/test/test-proc-cmdline.c +++ b/src/test/test-proc-cmdline.c @@ -30,8 +30,9 @@ static void test_proc_cmdline_override(void) { log_info("/* %s */", __func__); assert_se(putenv((char*) "SYSTEMD_PROC_CMDLINE=foo_bar=quux wuff-piep=tuet zumm some_arg_with_space='foo bar' and_one_more=\"zzz aaa\"") == 0); + assert_se(putenv((char*) "SYSTEMD_EFI_OPTIONS=differnt") == 0); - /* Test if the override works */ + /* First test if the overrides for /proc/cmdline still work */ _cleanup_free_ char *line = NULL, *value = NULL; assert_se(proc_cmdline(&line) >= 0); @@ -45,6 +46,19 @@ static void test_proc_cmdline_override(void) { assert_se(proc_cmdline_get_key("and_one_more", 0, &value) > 0 && streq_ptr(value, "zzz aaa")); value = mfree(value); + + assert_se(putenv((char*) "SYSTEMD_PROC_CMDLINE=") == 0); + assert_se(putenv((char*) "SYSTEMD_PROC_CMDLINE=foo_bar=quux wuff-piep=tuet zumm some_arg_with_space='foo bar' and_one_more=\"zzz aaa\"") == 0); + + assert_se(streq(line, "foo_bar=quux wuff-piep=tuet zumm some_arg_with_space='foo bar' and_one_more=\"zzz aaa\"")); + assert_se(proc_cmdline_get_key("foo_bar", 0, &value) > 0 && streq_ptr(value, "quux")); + value = mfree(value); + + assert_se(proc_cmdline_get_key("some_arg_with_space", 0, &value) > 0 && streq_ptr(value, "foo bar")); + value = mfree(value); + + assert_se(proc_cmdline_get_key("and_one_more", 0, &value) > 0 && streq_ptr(value, "zzz aaa")); + value = mfree(value); } static int parse_item_given(const char *key, const char *value, void *data) { @@ -140,6 +154,24 @@ static void test_proc_cmdline_get_bool(void) { log_info("/* %s */", __func__); assert_se(putenv((char*) "SYSTEMD_PROC_CMDLINE=foo_bar bar-waldo=1 x_y-z=0 quux=miep\nda=yes\nthe=1") == 0); + assert_se(putenv((char*) "SYSTEMD_EFI_OPTIONS=") == 0); + + assert_se(proc_cmdline_get_bool("", &value) == -EINVAL); + assert_se(proc_cmdline_get_bool("abc", &value) == 0 && value == false); + assert_se(proc_cmdline_get_bool("foo_bar", &value) > 0 && value == true); + assert_se(proc_cmdline_get_bool("foo-bar", &value) > 0 && value == true); + assert_se(proc_cmdline_get_bool("bar-waldo", &value) > 0 && value == true); + assert_se(proc_cmdline_get_bool("bar_waldo", &value) > 0 && value == true); + assert_se(proc_cmdline_get_bool("x_y-z", &value) > 0 && value == false); + assert_se(proc_cmdline_get_bool("x-y-z", &value) > 0 && value == false); + assert_se(proc_cmdline_get_bool("x-y_z", &value) > 0 && value == false); + assert_se(proc_cmdline_get_bool("x_y_z", &value) > 0 && value == false); + assert_se(proc_cmdline_get_bool("quux", &value) == -EINVAL && value == false); + assert_se(proc_cmdline_get_bool("da", &value) > 0 && value == true); + assert_se(proc_cmdline_get_bool("the", &value) > 0 && value == true); + + assert_se(putenv((char*) "SYSTEMD_PROC_CMDLINE=") == 0); + assert_se(putenv((char*) "SYSTEMD_EFI_OPTIONS=foo_bar bar-waldo=1 x_y-z=0 quux=miep\nda=yes\nthe=1") == 0); assert_se(proc_cmdline_get_bool("", &value) == -EINVAL); assert_se(proc_cmdline_get_bool("abc", &value) == 0 && value == false); From 28f0aef4c7e85f15346d68dcd6a08592af7c1662 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Aug 2019 17:16:39 +0200 Subject: [PATCH 09/10] man: document SystemdOptions variable --- man/kernel-command-line.xml | 9 ++++++--- man/systemd.xml | 14 +++++++------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/man/kernel-command-line.xml b/man/kernel-command-line.xml index f9408a028d..a79be5997d 100644 --- a/man/kernel-command-line.xml +++ b/man/kernel-command-line.xml @@ -27,9 +27,12 @@ Description - The kernel, the initial RAM disk (initrd) and - basic userspace functionality may be configured at boot via - kernel command line arguments. + The kernel, the initial RAM disk (initrd) and basic userspace functionality may be configured at + boot via kernel command line arguments. In addition, various systemd tools look at the EFI variable + SystemdOptions (if available). Both sources are combined, but the kernel command line + has higher priority. Please note that the EFI variable is only used by systemd tools, and is + ignored by the kernel and other user space tools, so it is not a replacement for the kernel + command line. For command line parameters understood by the kernel, please see diff --git a/man/systemd.xml b/man/systemd.xml index c01cf46e81..957d37dcd9 100644 --- a/man/systemd.xml +++ b/man/systemd.xml @@ -916,13 +916,13 @@ Kernel Command Line - When run as system instance systemd parses a number of - kernel command line argumentsIf run inside a Linux - container these arguments may be passed as command line arguments - to systemd itself, next to any of the command line options listed - in the Options section above. If run outside of Linux containers, - these arguments are parsed from /proc/cmdline - instead.: + When run as the system instance systemd parses a number of options listed below. They can be + specified as kernel command line argumentsIf run inside a Linux container these arguments + may be passed as command line arguments to systemd itself, next to any of the command line options listed + in the Options section above. If run outside of Linux containers, these arguments are parsed from + /proc/cmdline instead., or through the + SystemdOptions EFI variable (on EFI systems). The kernel command line has higher + priority. Following variables are understood: From 4e5aa79185d93d20defe1e371e33f89b86d93a3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Aug 2019 17:15:59 +0200 Subject: [PATCH 10/10] bootctl: add "system-options" verb --- man/bootctl.xml | 20 ++++++++++++----- man/kernel-command-line.xml | 3 ++- src/boot/bootctl.c | 45 ++++++++++++++++++++++++++++--------- 3 files changed, 51 insertions(+), 17 deletions(-) diff --git a/man/bootctl.xml b/man/bootctl.xml index 822d07a606..7ce41b70f9 100644 --- a/man/bootctl.xml +++ b/man/bootctl.xml @@ -133,6 +133,15 @@ and the firmware's boot loader list. + + + + Checks whether systemd-boot is installed in the ESP. Note that a + single ESP might host multiple boot loaders; this hence checks whether + systemd-boot is one (of possibly many) installed boot loaders — and neither + whether it is the default nor whether it is registered in any EFI variables. + + @@ -150,12 +159,13 @@ - + VALUE - Checks whether systemd-boot is installed in the ESP. Note that a - single ESP might host multiple boot loaders; this hence checks whether - systemd-boot is one (of possibly many) installed boot loaders — and neither - whether it is the default nor whether it is registered in any EFI variables. + When called without the optional argument, prints the current value of the + SystemdOptions EFI variable. When called with an argument, sets the + variable to that value. See + systemd1 + for the meaning of that variable. diff --git a/man/kernel-command-line.xml b/man/kernel-command-line.xml index a79be5997d..848f5ec443 100644 --- a/man/kernel-command-line.xml +++ b/man/kernel-command-line.xml @@ -452,7 +452,8 @@ systemd-backlight@.service8, systemd-rfkill.service8, systemd-hibernate-resume-generator8, - systemd-firstboot.service8 + systemd-firstboot.service8, + bootctl1 diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index 097c796a4e..2c8163360f 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -1052,8 +1052,9 @@ static int help(int argc, char *argv[], void *userdata) { " install Install systemd-boot to the ESP and EFI variables\n" " update Update systemd-boot in the ESP and EFI variables\n" " remove Remove systemd-boot from the ESP and EFI variables\n" - " random-seed Initialize random seed in ESP and EFI variables\n" " is-installed Test whether systemd-boot is installed in the ESP\n" + " random-seed Initialize random seed in ESP and EFI variables\n" + " system-options Query or set system options string in EFI variable\n" "\nBoot Loader Entries Commands:\n" " list List boot loader entries\n" " set-default ID Set default boot loader entry\n" @@ -1710,18 +1711,40 @@ static int verb_random_seed(int argc, char *argv[], void *userdata) { return 0; } +static int verb_system_options(int argc, char *argv[], void *userdata) { + int r; + + if (argc == 1) { + _cleanup_free_ char *line = NULL; + + r = efi_systemd_options_variable(&line); + if (r < 0) + return log_error_errno(r, "Failed to query SystemdOptions EFI variable: %m"); + + printf("SystemdOptions: %s\n", line); + + } else { + r = efi_set_variable_string(EFI_VENDOR_SYSTEMD, "SystemdOptions", argv[1]); + if (r < 0) + return log_error_errno(r, "Failed to set SystemdOptions EFI variable: %m"); + } + + return 0; +} + static int bootctl_main(int argc, char *argv[]) { static const Verb verbs[] = { - { "help", VERB_ANY, VERB_ANY, 0, help }, - { "status", VERB_ANY, 1, VERB_DEFAULT, verb_status }, - { "install", VERB_ANY, 1, 0, verb_install }, - { "update", VERB_ANY, 1, 0, verb_install }, - { "remove", VERB_ANY, 1, 0, verb_remove }, - { "random-seed", VERB_ANY, 1, 0, verb_random_seed }, - { "is-installed", VERB_ANY, 1, 0, verb_is_installed }, - { "list", VERB_ANY, 1, 0, verb_list }, - { "set-default", 2, 2, 0, verb_set_default }, - { "set-oneshot", 2, 2, 0, verb_set_default }, + { "help", VERB_ANY, VERB_ANY, 0, help }, + { "status", VERB_ANY, 1, VERB_DEFAULT, verb_status }, + { "install", VERB_ANY, 1, 0, verb_install }, + { "update", VERB_ANY, 1, 0, verb_install }, + { "remove", VERB_ANY, 1, 0, verb_remove }, + { "is-installed", VERB_ANY, 1, 0, verb_is_installed }, + { "list", VERB_ANY, 1, 0, verb_list }, + { "set-default", 2, 2, 0, verb_set_default }, + { "set-oneshot", 2, 2, 0, verb_set_default }, + { "random-seed", VERB_ANY, 1, 0, verb_random_seed }, + { "system-options", VERB_ANY, 2, 0, verb_system_options }, {} };