From 058cbd2aba360a933d07f674002257118dd2bf9e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 10 Jul 2026 10:53:12 +0200 Subject: [PATCH 1/6] sysupdate: explicitly refuse --root=/--image= in conjunction with --definitions= This is currently effectively not supported (as we sometimes prefix the definitions path with root and sometimes not), let's make this official for now and refuse it. We could in theory support the combination but I don't see the big benefit of it for now, hence let's just refuse it. --- src/sysupdate/sysupdate.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index fe39d31f6a..c31e711795 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -2636,8 +2636,13 @@ static int parse_argv(int argc, char *argv[], char ***remaining_args) { if (arg_image && arg_root) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Please specify either --root= or --image=, the combination of both is not supported."); - if ((arg_image || arg_root) && arg_reboot) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --reboot switch may not be combined with --root= or --image=."); + if (arg_image || arg_root) { + if (arg_reboot) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --reboot switch may not be combined with --root= or --image=."); + + if (arg_definitions) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --definitions= switch may not be combined with --root= or --image=."); + } if (arg_reboot && arg_component) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "The --reboot switch may not be combined with --component=, as automatic reboots only apply to the booted OS version."); From d298f9bc8e735ad14eef6c989d50c21a241bbe55 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 10 Jul 2026 14:34:59 +0200 Subject: [PATCH 2/6] sysupdate: generalize feature name validity check Let's switch to string_is_safe(), and make this available to the rest of the sysupdate sources too. This both relaxes and tighten the rules slightly. i.e. control character and stuff are no longer allowed, but valid UTF-8 (as opposed to ASCII) now is. --- src/sysupdate/meson.build | 2 +- src/sysupdate/sysupdate-util.c | 11 +++++++++++ src/sysupdate/sysupdate-util.h | 2 ++ src/sysupdate/sysupdated.c | 19 ++----------------- 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/sysupdate/meson.build b/src/sysupdate/meson.build index 664f439f79..c4f569096f 100644 --- a/src/sysupdate/meson.build +++ b/src/sysupdate/meson.build @@ -39,7 +39,7 @@ executables += [ 'name' : 'systemd-sysupdated', 'dbus' : true, 'conditions' : ['ENABLE_SYSUPDATED'], - 'sources' : files('sysupdated.c', 'sysupdate-target.c'), + 'sources' : files('sysupdated.c', 'sysupdate-target.c', 'sysupdate-util.c'), }, executable_template + { 'name' : 'updatectl', diff --git a/src/sysupdate/sysupdate-util.c b/src/sysupdate/sysupdate-util.c index 8b0a4924e4..579c107154 100644 --- a/src/sysupdate/sysupdate-util.c +++ b/src/sysupdate/sysupdate-util.c @@ -31,6 +31,17 @@ int reboot_now(void) { return 0; } +bool feature_name_valid(const char *c) { + + if (!string_is_safe(c, STRING_FILENAME_PART)) + return false; + + /* Stack allocation is safe, since STRING_FILENAME_PART includes a length check */ + const char *j = strjoina(c, ".feature.d"); + + return filename_is_valid(j); +} + bool component_name_valid(const char *c) { /* See if the specified string enclosed in the directory prefix+suffix would be a valid file name */ diff --git a/src/sysupdate/sysupdate-util.h b/src/sysupdate/sysupdate-util.h index 07d9def708..6d5230f1e3 100644 --- a/src/sysupdate/sysupdate-util.h +++ b/src/sysupdate/sysupdate-util.h @@ -8,5 +8,7 @@ int reboot_now(void); #define SD_SYSUPDATE_OFFLINE (UINT64_C(1) << 0) #define SD_SYSUPDATE_FLAGS_ALL (SD_SYSUPDATE_OFFLINE) +bool feature_name_valid(const char *c); + bool component_name_valid(const char *c); int get_component_list(const char *root, char ***ret); diff --git a/src/sysupdate/sysupdated.c b/src/sysupdate/sysupdated.c index 8022fde77e..bc5a2439b5 100644 --- a/src/sysupdate/sysupdated.c +++ b/src/sysupdate/sysupdated.c @@ -32,7 +32,6 @@ #include "notify-recv.h" #include "os-util.h" #include "parse-util.h" -#include "path-util.h" #include "pidref.h" #include "process-util.h" #include "runtime-scope.h" @@ -42,7 +41,6 @@ #include "strv.h" #include "sysupdate-target.h" #include "sysupdate-util.h" -#include "utf8.h" #define FEATURES_DROPIN_NAME "systemd-sysupdate-enabled" @@ -1404,19 +1402,6 @@ static int target_method_list_features(sd_bus_message *msg, void *userdata, sd_b return sd_bus_message_send(reply); } -static bool feature_name_is_valid(const char *name) { - if (isempty(name)) - return false; - - if (!ascii_is_valid(name)) - return false; - - if (!filename_is_valid(strjoina(name, ".feature.d"))) - return false; - - return true; -} - static int target_method_describe_feature(sd_bus_message *msg, void *userdata, sd_bus_error *error) { Target *t = ASSERT_PTR(userdata); _cleanup_(job_freep) Job *j = NULL; @@ -1430,7 +1415,7 @@ static int target_method_describe_feature(sd_bus_message *msg, void *userdata, s if (r < 0) return r; - if (!feature_name_is_valid(feature)) + if (!feature_name_valid(feature)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid feature name"); if (flags != 0) @@ -1470,7 +1455,7 @@ static int target_method_set_feature_enabled(sd_bus_message *msg, void *userdata r = sd_bus_message_read(msg, "sit", &feature, &enabled, &flags); if (r < 0) return r; - if (!feature_name_is_valid(feature)) + if (!feature_name_valid(feature)) return sd_bus_reply_method_errorf(msg, SD_BUS_ERROR_INVALID_ARGS, "The specified feature is invalid"); From 0c7992f67489e8ae9c60c819b0058ffd2664e3c3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Jun 2026 12:35:01 +0200 Subject: [PATCH 3/6] sysupdate: add enable-feature/disable-feature verbs These do what updatectl's verbs of the same name does, but are implemented as low-level concepts in sysupdate itself. (The idea is to eventually wrap this in Varlink IPC, and make updatectl use them that way.) --- src/sysupdate/sysupdate.c | 89 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index c31e711795..6176143518 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -15,6 +15,7 @@ #include "discover-image.h" #include "dissect-image.h" #include "dlopen-note.h" +#include "dropin.h" #include "env-util.h" #include "errno-util.h" #include "fd-util.h" @@ -33,6 +34,7 @@ #include "pager.h" #include "parse-argument.h" #include "parse-util.h" +#include "path-util.h" #include "pretty-print.h" #include "runtime-scope.h" #include "sort-util.h" @@ -1945,6 +1947,93 @@ static int verb_features(int argc, char *argv[], uintptr_t _data, void *userdata return 0; } +static int make_dropin_dir(Context *c, char **ret) { + _cleanup_free_ char *dir = NULL; + + assert(c); + assert(ret); + + /* Returns the (writable) directory where feature definitions live, so that we can drop our + * 'Enabled=' override right next to them. This mirrors the directory logic in + * context_read_definitions(), but settles on a single writable location below /etc. */ + + if (c->definitions) + dir = strdup(c->definitions); /* --root= is not supported for this for now */ + else if (c->component) { + _cleanup_free_ char *n = strjoin("sysupdate.", c->component, ".d"); + if (!n) + return log_oom(); + + dir = path_join(c->root, SYSCONF_DIR, n); + } else + dir = path_join(c->root, SYSCONF_DIR "/sysupdate.d"); + if (!dir) + return log_oom(); + + *ret = TAKE_PTR(dir); + return 0; +} + +VERB(verb_enable_feature, "enable-feature", "FEATURE…", 2, VERB_ANY, 0, + "Enable optional feature"); +VERB(verb_enable_feature, "disable-feature", "FEATURE…", 2, VERB_ANY, 0, + "Disable optional feature"); +static int verb_enable_feature(int argc, char *argv[], uintptr_t _data, void *userdata) { + bool enable = streq(argv[0], "enable-feature"); + int r; + + _cleanup_(context_done) Context context = CONTEXT_NULL; + r = context_from_cmdline(&context); + if (r < 0) + return r; + + if (context.component_all) + return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "--component-all currently not supported for '%s'.", argv[0]); + + r = context_load_offline( + &context, + /* process_image_flags= */ 0, + /* read_definitions_flags= */ 0); + if (r < 0) + return r; + + _cleanup_free_ char *dropin_dir = NULL; + r = make_dropin_dir(&context, &dropin_dir); + if (r < 0) + return r; + + STRV_FOREACH(name, strv_skip(argv, 1)) { + + if (!feature_name_valid(*name)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Feature name invalid: %s", *name); + + if (!hashmap_contains(context.features, *name)) + return log_error_errno(SYNTHETIC_ERRNO(ENOENT), + "Optional feature not found: %s", *name); + + _cleanup_free_ char *fname = strjoin(*name, ".feature"); + if (!fname) + return log_oom(); + + /* We assume that no sysadmin will name their config 50-systemd-sysupdate-enabled.conf */ + r = write_drop_in_format( + dropin_dir, + fname, + 50, "systemd-sysupdate-enabled", + "# Generated via 'systemd-sysupdate %s'\n\n" + "[Feature]\n" + "Enabled=%s\n", + argv[0], + yes_no(enable)); + if (r < 0) + return log_error_errno(r, "Failed to write drop-in for feature '%s': %m", *name); + + log_info("Feature '%s' %s.", *name, enable ? "enabled" : "disabled"); + } + + return 0; +} + VERB_NOARG(verb_check_new, "check-new", "Check if there's a new version available"); static int verb_check_new(int argc, char *argv[], uintptr_t _data, void *userdata) { From 779c328a7930b08602eed0a9bba963c6c576e9b1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Jun 2026 12:50:29 +0200 Subject: [PATCH 4/6] sysupdate: add new enable-component/disable-component verb This mimics the enable-feature/disable-feature verbs, but operates on whole components, not features. --- src/sysupdate/sysupdate.c | 70 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 6176143518..54c2673569 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -2480,6 +2480,76 @@ static int verb_components(int argc, char *argv[], uintptr_t _data, void *userda return 0; } +VERB(verb_enable_component, "enable-component", "COMPONENT…", 2, VERB_ANY, 0, + "Enable component"); +VERB(verb_enable_component, "disable-component", "COMPONENT…", 2, VERB_ANY, 0, + "Disable component"); +static int verb_enable_component(int argc, char *argv[], uintptr_t _data, void *userdata) { + bool enable = streq(argv[0], "enable-component"); + int r; + + _cleanup_(context_done) Context context = CONTEXT_NULL; + r = context_from_cmdline(&context); + if (r < 0) + return r; + + if (context.component || context.component_all) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "The --component= and --component-all switches may not be combined with '%s', " + "specify the component(s) as arguments instead.", argv[0]); + + if (context.definitions) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "The --definitions= switch may not be combined with '%s'.", argv[0]); + + r = context_load_offline( + &context, + /* process_image_flags= */ 0, + /* read_definitions_flags= */ 0); + if (r < 0) + return r; + + _cleanup_strv_free_ char **component_names = NULL; + r = context_list_components(&context, &component_names, /* ret_has_default_component= */ NULL); + if (r < 0) + return r; + + /* Component definition files live directly below the configuration directories, hence the drop-in + * goes right next to them below /etc. */ + _cleanup_free_ char *dropin_dir = path_join(context.root, SYSCONF_DIR); + if (!dropin_dir) + return log_oom(); + + STRV_FOREACH(name, strv_skip(argv, 1)) { + if (!component_name_valid(*name)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Component name invalid: %s", *name); + + if (!strv_contains(component_names, *name)) + return log_error_errno(SYNTHETIC_ERRNO(ENOENT), "Component not found: %s", *name); + + _cleanup_free_ char *fname = strjoin("sysupdate.", *name, ".component"); + if (!fname) + return log_oom(); + + /* We assume that no sysadmin will name their config 50-systemd-sysupdate-enabled.conf */ + r = write_drop_in_format( + dropin_dir, + fname, + 50, "systemd-sysupdate-enabled", + "# Generated via 'systemd-sysupdate %s'\n\n" + "[Component]\n" + "Enabled=%s\n", + argv[0], + yes_no(enable)); + if (r < 0) + return log_error_errno(r, "Failed to write drop-in for component '%s': %m", *name); + + log_info("Component '%s' %s.", *name, enable ? "enabled" : "disabled"); + } + + return 0; +} + VERB_NOARG(verb_cleanup, "cleanup", "Clean up orphaned files"); static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(context_done) Context context = CONTEXT_NULL; From 14d914ac7f3056a161fb91c6b5be27acc9fcbc54 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 10 Jul 2026 14:36:31 +0200 Subject: [PATCH 5/6] sysupdate: tighten component_name_valid() mildly Let's make sure that we can still generate a component enablement drop-in for any component that might be defined. --- src/sysupdate/sysupdate-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sysupdate/sysupdate-util.c b/src/sysupdate/sysupdate-util.c index 579c107154..32b46b8dab 100644 --- a/src/sysupdate/sysupdate-util.c +++ b/src/sysupdate/sysupdate-util.c @@ -52,7 +52,7 @@ bool component_name_valid(const char *c) { return false; /* Stack allocation is safe, since STRING_FILENAME_PART includes a length check */ - const char *j = strjoina("sysupdate.", c, ".d"); + const char *j = strjoina("sysupdate.", c, ".component.d"); return filename_is_valid(j); } From 0d889c48f5664e73ef22b081a7ce1dae3c40812f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 24 Jun 2026 12:52:09 +0200 Subject: [PATCH 6/6] sysupdate: move cleanup verb up close to "vacuum" These are philosophically similar concepts: one deletes old versions based on whether they are old, and the other deletes old files based on whether they are orphaned, let's list them together. --- src/sysupdate/sysupdate.c | 114 +++++++++++++++++++------------------- 1 file changed, 57 insertions(+), 57 deletions(-) diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 54c2673569..56f0281162 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -2282,6 +2282,63 @@ static int verb_vacuum(int argc, char *argv[], uintptr_t _data, void *userdata) return context_vacuum(&context, 0, NULL); } +VERB_NOARG(verb_cleanup, "cleanup", "Clean up orphaned files"); +static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) { + _cleanup_(context_done) Context context = CONTEXT_NULL; + int r; + + assert(argc <= 1); + + r = context_from_cmdline(&context); + if (r < 0) + return r; + + if (context.cleanup == 0) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invocation of 'cleanup' with --cleanup=no is contradictory, refusing."); + + r = context_load_offline( + &context, + /* process_image_flags= */ 0, + /* read_definitions_flags= */ 0); + if (r < 0) + return r; + + int ret = 0; + RET_GATHER(ret, installdb_cleanup_component(&context)); + + if (context.component_all) { + _cleanup_strv_free_ char **z = NULL; + r = installdb_list_components(&context, &z); + if (r < 0) + return log_error_errno(r, "Failed to enumerate components: %m"); + + STRV_FOREACH(i, z) { + _cleanup_(context_done) Context component_context = CONTEXT_NULL; + + r = context_from_cmdline(&component_context); + if (r < 0) + return r; + + /* Override the component with our iter. This needs to be done in a fresh Context + * as the installdb_fd and other state are specific to the component. */ + r = free_and_strdup_warn(&component_context.component, *i); + if (r < 0) + return r; + + r = context_load_offline( + &component_context, + /* process_image_flags= */ 0, + /* read_definitions_flags= */ 0); + if (r < 0) + return r; + + RET_GATHER(ret, installdb_cleanup_component(&component_context)); + } + } + + return ret; +} + VERB(verb_pending_or_reboot, "pending", NULL, 1, 1, 0, "Report whether a newer version is installed than currently booted"); VERB(verb_pending_or_reboot, "reboot", NULL, 1, 1, 0, @@ -2550,63 +2607,6 @@ static int verb_enable_component(int argc, char *argv[], uintptr_t _data, void * return 0; } -VERB_NOARG(verb_cleanup, "cleanup", "Clean up orphaned files"); -static int verb_cleanup(int argc, char *argv[], uintptr_t _data, void *userdata) { - _cleanup_(context_done) Context context = CONTEXT_NULL; - int r; - - assert(argc <= 1); - - r = context_from_cmdline(&context); - if (r < 0) - return r; - - if (context.cleanup == 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invocation of 'cleanup' with --cleanup=no is contradictory, refusing."); - - r = context_load_offline( - &context, - /* process_image_flags= */ 0, - /* read_definitions_flags= */ 0); - if (r < 0) - return r; - - int ret = 0; - RET_GATHER(ret, installdb_cleanup_component(&context)); - - if (context.component_all) { - _cleanup_strv_free_ char **z = NULL; - r = installdb_list_components(&context, &z); - if (r < 0) - return log_error_errno(r, "Failed to enumerate components: %m"); - - STRV_FOREACH(i, z) { - _cleanup_(context_done) Context component_context = CONTEXT_NULL; - - r = context_from_cmdline(&component_context); - if (r < 0) - return r; - - /* Override the component with our iter. This needs to be done in a fresh Context - * as the installdb_fd and other state are specific to the component. */ - r = free_and_strdup_warn(&component_context.component, *i); - if (r < 0) - return r; - - r = context_load_offline( - &component_context, - /* process_image_flags= */ 0, - /* read_definitions_flags= */ 0); - if (r < 0) - return r; - - RET_GATHER(ret, installdb_cleanup_component(&component_context)); - } - } - - return ret; -} - static int help(void) { _cleanup_(table_unrefp) Table *common_options = NULL, *options = NULL, *verbs = NULL; int r;