From f351668cbef34852069b32eb82d95b5649f0135d Mon Sep 17 00:00:00 2001 From: Kai Lueke Date: Thu, 11 Dec 2025 19:49:20 +0900 Subject: [PATCH] sysext: Fix config file support with --root= Config files for --root= weren't picked up as expected because the --root= flag got parsed after the config file. Switch the order of config file and CLI flag parsing while letting the CLI flags overwrite things set by the config files by tracking state during parsing. --- src/sysext/sysext.c | 42 +++++++++++++---- test/units/TEST-50-DISSECT.sysext.sh | 69 ++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 9 deletions(-) diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index ab1f69f8c9b..d18f1389370 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -93,8 +93,10 @@ static bool arg_force = false; static bool arg_no_reload = false; static int arg_noexec = -1; static ImagePolicy *arg_image_policy = NULL; +static bool arg_image_policy_set = false; /* Tracks initialization */ static bool arg_varlink = false; static MutableMode arg_mutable = MUTABLE_NO; +static bool arg_mutable_set = false; /* Tracks initialization */ static const char *arg_overlayfs_mount_options = NULL; /* Is set to IMAGE_CONFEXT when systemd is called with the confext functionality instead of the default */ @@ -163,10 +165,13 @@ static int parse_mutable_mode(const char *p) { static DEFINE_CONFIG_PARSE_ENUM(config_parse_mutable_mode, mutable_mode, MutableMode); static int parse_config_file(ImageClass image_class) { + _cleanup_(image_policy_freep) ImagePolicy *config_image_policy = NULL; + MutableMode config_mutable = MUTABLE_NO; const char *section = image_class == IMAGE_SYSEXT ? "SysExt" : "ConfExt"; + const char *sections = image_class == IMAGE_SYSEXT ? "SysExt\0" : "ConfExt\0"; const ConfigTableItem items[] = { - { section, "Mutable", config_parse_mutable_mode, 0, &arg_mutable }, - { section, "ImagePolicy", config_parse_image_policy, 0, &arg_image_policy }, + { section, "Mutable", config_parse_mutable_mode, 0, &config_mutable }, + { section, "ImagePolicy", config_parse_image_policy, 0, &config_image_policy }, {} }; _cleanup_free_ char *config_file = NULL; @@ -179,7 +184,7 @@ static int parse_config_file(ImageClass image_class) { r = config_parse_standard_file_with_dropins_full( arg_root, config_file, - image_class == IMAGE_SYSEXT ? "SysExt\0" : "ConfExt\0", + sections, config_item_table_lookup, items, CONFIG_PARSE_WARN, /* userdata = */ NULL, @@ -188,6 +193,17 @@ static int parse_config_file(ImageClass image_class) { if (r < 0) return r; + /* Because this runs after parse_argv we only overwrite when things aren't set yet. */ + if (!arg_mutable_set) { + arg_mutable = config_mutable; + arg_mutable_set = true; + } + + if (!arg_image_policy_set) { + arg_image_policy = TAKE_PTR(config_image_policy); + arg_image_policy_set = true; + } + return 0; } @@ -2592,6 +2608,9 @@ static int parse_argv(int argc, char *argv[]) { r = parse_image_policy_argument(optarg, &arg_image_policy); if (r < 0) return r; + /* When the CLI flag is given we initialize even if NULL + * so that the config file entry won't overwrite it */ + arg_image_policy_set = true; break; case ARG_NOEXEC: @@ -2618,6 +2637,7 @@ static int parse_argv(int argc, char *argv[]) { if (r < 0) return log_error_errno(r, "Failed to parse argument to --mutable=: %s", optarg); arg_mutable = r; + arg_mutable_set = true; break; case '?': @@ -2646,8 +2666,10 @@ static int parse_env(void) { if (r < 0) log_warning("Failed to parse %s environment variable value '%s'. Ignoring.", image_class_info[arg_image_class].mode_env, env_var); - else + else { arg_mutable = r; + arg_mutable_set = true; + } } env_var = secure_getenv(image_class_info[arg_image_class].opts_env); @@ -2691,16 +2713,18 @@ static int run(int argc, char *argv[]) { if (r < 0) return r; - /* Parse configuration file */ - r = parse_config_file(arg_image_class); - if (r < 0) - log_warning_errno(r, "Failed to parse global config file, ignoring: %m"); - /* Parse command line */ r = parse_argv(argc, argv); if (r <= 0) return r; + /* Parse configuration file after argv because it needs --root=. + * The config entries will not overwrite values set already by + * env/argv because we track initialization. */ + r = parse_config_file(arg_image_class); + if (r < 0) + log_warning_errno(r, "Failed to parse global config file, ignoring: %m"); + if (arg_varlink) { _cleanup_(sd_varlink_server_unrefp) sd_varlink_server *varlink_server = NULL; diff --git a/test/units/TEST-50-DISSECT.sysext.sh b/test/units/TEST-50-DISSECT.sysext.sh index 3668e66e81b..c9ba8939859 100755 --- a/test/units/TEST-50-DISSECT.sysext.sh +++ b/test/units/TEST-50-DISSECT.sysext.sh @@ -163,6 +163,24 @@ prepare_extension_image_with_matching_id_like() { prepend_trap "rm -rf ${ext_dir@Q}" } +prepare_extension_image_raw() { + local root=${1:-} + local hierarchy=${2:?} + local ext_dir ext_release name + + name="test-extension" + ext_dir="$root/var/lib/extensions/$name" + ext_release="$ext_dir/usr/lib/extension-release.d/extension-release.$name" + mkdir -p "${ext_release%/*}" + echo "ID=_any" >"$ext_release" + mkdir -p "$ext_dir/$hierarchy" + touch "$ext_dir$hierarchy/preexisting-file-in-extension-image" + mksquashfs "$ext_dir" "$ext_dir.raw" -all-root -noappend -quiet + rm -rf "$ext_dir" + + prepend_trap "rm -rf ${ext_dir@Q}.raw" +} + prepare_extension_mutable_dir() { local dir=${1:?} @@ -1170,6 +1188,57 @@ fi rm "${fake_root}/etc/initrd-release" ) +( init_trap +: "Check config file support for --root=" +fake_root=${roots_dir:+"$roots_dir/config-file"} +hierarchy=/opt +extension_data_dir="$fake_root/var/lib/extensions.mutable$hierarchy" + +[[ "$FSTYPE" == "fuseblk" ]] && exit 0 +if [ "$roots_dir" = "" ]; then + echo >&2 "Skipping test when --root= is not used" + exit 0 +fi + +prepare_root "$fake_root" "$hierarchy" +prepare_extension_image_raw "$fake_root" "$hierarchy" +prepare_extension_mutable_dir "$extension_data_dir" +prepare_read_only_hierarchy "$fake_root" "$hierarchy" + +mkdir -p "$fake_root/etc/systemd/" +{ echo "[SysExt]" ; echo "Mutable=auto" ; } > "$fake_root/etc/systemd/sysext.conf" +# Config file should be picked up with --root= set +run_systemd_sysext "$fake_root" merge +MNTOPT=$(findmnt "$fake_root$hierarchy" --first-only --direction backward --raw --noheadings -o VFS-OPTIONS | grep -o rw || true) +if [ "$MNTOPT" != "rw" ]; then + echo >&2 "Merge did not pick up mutable setting from config file" + exit 1 +fi +extension_verify_after_merge "$fake_root" "$hierarchy" -e -h -u +run_systemd_sysext "$fake_root" unmerge + +# CLI arg should be able to overwrite config file +run_systemd_sysext "$fake_root" merge --mutable=no +MNTOPT=$(findmnt "$fake_root$hierarchy" --first-only --direction backward --raw --noheadings -o VFS-OPTIONS | grep -o ro || true) +if [ "$MNTOPT" != "ro" ]; then + echo >&2 "Merge did not pick up CLI arg to overwrite mutable setting from config file" + exit 1 +fi +extension_verify_after_merge "$fake_root" "$hierarchy" -e -h +run_systemd_sysext "$fake_root" unmerge + +{ echo "[SysExt]" ; echo "ImagePolicy=root=signed+absent:usr=signed+absent" ; } > "$fake_root/etc/systemd/sysext.conf" +# Config file should be picked up with --root= set +if run_systemd_sysext "$fake_root" merge; then + echo >&2 "Merge did not fail with strict image policy in config file" + exit 1 +fi +# CLI arg should be able to overwrite config file +run_systemd_sysext "$fake_root" merge --image-policy="*" +extension_verify_after_merge "$fake_root" "$hierarchy" -e -h +run_systemd_sysext "$fake_root" unmerge +) + } # End of run_sysext_tests -- 2.47.3