diff --git a/man/systemd-sysext.xml b/man/systemd-sysext.xml index a0e77ad729..e9d275fac4 100644 --- a/man/systemd-sysext.xml +++ b/man/systemd-sysext.xml @@ -134,7 +134,10 @@ page for a simple mechanism for shipping system services in disk images, in a similar fashion to OS extensions. Note the differences in isolation between these two mechanisms: while system extensions directly extend the underlying OS image with additional files that appear as if they were shipped in the OS image itself - and thus imply no security isolation, portable services imply service-level sandboxing in one way or another. + and thus imply no security isolation, portable services imply service-level sandboxing in one way or another. + Binaries shipped in system extensions can only link against libraries from the host if the extension is + bound to the version of the host OS (see further below) or otherwise have to use static linking. + Portable services, however, are fully decoupled from the host as they ship their own library dependencies. The systemd-sysext.service and systemd-confext.service services are guaranteed to finish start-up before basic.target is reached; i.e., by the time @@ -170,9 +173,22 @@ but the used architecture identifiers are the same as for ConditionArchitecture= described in systemd.unit5. EXTENSION_RELOAD_MANAGER= can be set to 1 if the extension requires a service manager reload after application - of the extension. Note that for the reasons mentioned earlier, - Portable Services remain - the recommended way to ship system services. + of the extension. EXTENSION_RESTART_UNITS= and + EXTENSION_RELOAD_OR_RESTART_UNITS= can be set to a whitespace-separated list of unit + names (e.g. EXTENSION_RESTART_UNITS="my.service other.service") that + systemd-sysext/systemd-confext should restart resp. reload (or + restart, if the unit does not support reloading) after merging or refreshing the extension, and after + unmerging it. Use EXTENSION_RESTART_UNITS= for units whose binary the extension + replaces (a reload would not pick up the new binary), and EXTENSION_RELOAD_OR_RESTART_UNITS= + for units the extension extends with configuration drop-ins only (so reload-capable services are not + needlessly interrupted). If a unit has disappeared, e.g., on unmerge because it was part of the + extension, it is stopped instead. If the same unit is listed in both fields, + EXTENSION_RESTART_UNITS= takes precedence. Listing any unit in either field implies + EXTENSION_RELOAD_MANAGER=1, as the service manager would otherwise not yet (or no + longer) see the unit files shipped by the extension. Note that these settings do not take effect when + the extension is merged for the system already from the initrd and extensions should still ship a + .wants/ or .upholds/ symlink from the unit's install target to + ensure the unit starts at boot. System extensions should not ship a /usr/lib/os-release file (as that would be merged into the host /usr/ tree, overriding the host OS version data, which is not desirable). @@ -501,7 +517,9 @@ When used with merge, unmerge or refresh, do not reload daemon after executing the changes even if an extension that is applied requires a reload via the - EXTENSION_RELOAD_MANAGER= set to 1. + EXTENSION_RELOAD_MANAGER= set to 1. This also skips processing any units + listed via EXTENSION_RESTART_UNITS= or + EXTENSION_RELOAD_OR_RESTART_UNITS=. diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index a617bd0f76..0f4102d850 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -15,9 +15,12 @@ #include "blkid-util.h" #include "blockdev-util.h" #include "build.h" +#include "bus-common-errors.h" +#include "bus-locator.h" #include "bus-polkit.h" #include "bus-unit-util.h" #include "bus-util.h" +#include "bus-wait-for-jobs.h" #include "capability-util.h" #include "chase.h" #include "conf-parser.h" @@ -57,12 +60,14 @@ #include "rm-rf.h" #include "runtime-scope.h" #include "selinux-util.h" +#include "set.h" #include "sort-util.h" #include "stat-util.h" #include "string-table.h" #include "string-util.h" #include "strv.h" #include "time-util.h" +#include "unit-name.h" #include "varlink-io.systemd.sysext.h" #include "varlink-util.h" #include "verbs.h" @@ -289,16 +294,60 @@ static int is_our_mount_point( return true; } -static int need_reload( - ImageClass image_class, - char **hierarchies, - bool no_reload) { - - /* Parse the mounted images to find out if we need to reload the daemon. */ +static int split_unit_string(const char *s, const char *field, const char *extension, Set **units) { + _cleanup_strv_free_ char **split = NULL; int r; - if (no_reload) - return false; + assert(field); + assert(extension); + assert(units); + + if (isempty(s)) + return 0; + + split = strv_split(s, /* separators= whitespace */ NULL); + if (!split) + return log_oom(); + + STRV_FOREACH(u, split) { + if (!unit_name_is_valid(*u, UNIT_NAME_PLAIN|UNIT_NAME_INSTANCE)) { + log_warning("Invalid unit name '%s' in %s= of %s, ignoring.", *u, field, extension); + continue; + } + + r = set_put_strdup(units, *u); + if (r < 0) + return log_oom(); + } + + return 0; +} + +static int get_extension_release_metadata( + ImageClass image_class, + char **hierarchies, + bool no_reload, + bool *ret_need_reload, + Set **ret_restart_units, + Set **ret_reload_or_restart_units) { + + /* Parse the mounted images to find out if we need to reload the daemon, and which units to + * restart or reload-or-restart afterwards. */ + _cleanup_set_free_ Set *restart_units = NULL, *reload_or_restart_units = NULL; + bool need_to_reload = false; + int r; + + if (no_reload || !isempty(arg_root)) { + /* With --root= we'd be talking to the host service manager about extensions merged + * into a foreign root, which is never what we want. */ + if (ret_need_reload) + *ret_need_reload = false; + if (ret_restart_units) + *ret_restart_units = NULL; + if (ret_reload_or_restart_units) + *ret_reload_or_restart_units = NULL; + return 0; + } STRV_FOREACH(p, hierarchies) { _cleanup_free_ char *f = NULL, *buf = NULL, *resolved = NULL; @@ -334,8 +383,7 @@ static int need_reload( STRV_FOREACH(extension, mounted_extensions) { _cleanup_strv_free_ char **extension_release = NULL; - const char *extension_reload_manager = NULL; - int b; + const char *value; r = load_extension_release_pairs(arg_root, image_class, *extension, /* relax_extension_release_check= */ true, &extension_release); if (r < 0) { @@ -343,23 +391,50 @@ static int need_reload( continue; } - extension_reload_manager = strv_env_pairs_get(extension_release, "EXTENSION_RELOAD_MANAGER"); - if (isempty(extension_reload_manager)) - continue; - - b = parse_boolean(extension_reload_manager); - if (b < 0) { - log_warning_errno(b, "Failed to parse the extension metadata to know if the manager needs to be reloaded, ignoring: %m"); - continue; + value = strv_env_pairs_get(extension_release, "EXTENSION_RELOAD_MANAGER"); + if (!isempty(value)) { + int b = parse_boolean(value); + if (b < 0) + log_warning_errno(b, "Failed to parse EXTENSION_RELOAD_MANAGER= of %s, ignoring: %m", *extension); + else if (b) + need_to_reload = true; } - if (b) - /* If at least one extension wants a reload, we reload. */ - return true; + if (ret_restart_units) { + value = strv_env_pairs_get(extension_release, "EXTENSION_RESTART_UNITS"); + r = split_unit_string(value, "EXTENSION_RESTART_UNITS", *extension, &restart_units); + if (r < 0) + return r; + if (!isempty(value)) + /* Listing units to restart implies a manager reload because a unit + * shipped (and later removed) by the extension would otherwise not + * be visible to the manager when RestartUnit/StopUnit is issued. */ + need_to_reload = true; + } + + if (ret_reload_or_restart_units) { + value = strv_env_pairs_get(extension_release, "EXTENSION_RELOAD_OR_RESTART_UNITS"); + r = split_unit_string(value, "EXTENSION_RELOAD_OR_RESTART_UNITS", *extension, &reload_or_restart_units); + if (r < 0) + return r; + if (!isempty(value)) + need_to_reload = true; + } } } - return false; + /* If a unit is listed in both, restart takes precedence over reload-or-restart. */ + const char *u; + SET_FOREACH(u, restart_units) + free(set_remove(reload_or_restart_units, u)); + + if (ret_need_reload) + *ret_need_reload = need_to_reload; + if (ret_restart_units) + *ret_restart_units = TAKE_PTR(restart_units); + if (ret_reload_or_restart_units) + *ret_reload_or_restart_units = TAKE_PTR(reload_or_restart_units); + return 0; } static int move_submounts(const char *src, const char *dst) { @@ -425,6 +500,77 @@ static int daemon_reload(void) { return bus_service_manager_reload(bus); } +static int dispatch_unit_method(sd_bus *bus, BusWaitForJobs *w, Set *units, const char *method) { + const char *unit; + int r; + + assert(bus); + assert(w); + assert(method); + + /* Issue the given method for each unit and wait for the jobs to complete. Fall back to StopUnit + * when the call reports that the unit's file is gone, e.g., on unmerge when the unit was part of + * the extension, because otherwise the running service is leaked. */ + + SET_FOREACH(unit, units) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + const char *m = method, *job; + + r = bus_call_method(bus, bus_systemd_mgr, m, &error, &reply, "ss", unit, "replace"); + if (r < 0 && sd_bus_error_has_names(&error, BUS_ERROR_NO_SUCH_UNIT, BUS_ERROR_LOAD_FAILED)) { + sd_bus_error_free(&error); + m = "StopUnit"; + r = bus_call_method(bus, bus_systemd_mgr, m, &error, &reply, "ss", unit, "replace"); + if (r < 0 && sd_bus_error_has_name(&error, BUS_ERROR_NO_SUCH_UNIT)) { + log_debug("Unit '%s' is already gone, nothing to stop.", unit); + continue; + } + } + if (r < 0) { + log_warning("Failed to %s unit '%s': %s", m, unit, bus_error_message(&error, r)); + continue; + } + + r = sd_bus_message_read(reply, "o", &job); + if (r < 0) + return bus_log_parse_error(r); + + r = bus_wait_for_jobs_one(w, job, BUS_WAIT_JOBS_LOG_ERROR, /* extra_args= */ NULL); + if (r < 0) + log_warning_errno(r, "Failed to wait for %s job of unit '%s', ignoring: %m", m, unit); + } + + return 0; +} + +static int restart_units(Set *restart_units, Set *reload_or_restart_units) { + _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *w = NULL; + int r; + + if (set_isempty(restart_units) && set_isempty(reload_or_restart_units)) + return 0; + + r = bus_connect_system_systemd(&bus); + if (r < 0) + return log_error_errno(r, "Failed to get D-Bus connection: %m"); + + r = bus_wait_for_jobs_new(bus, &w); + if (r < 0) + return log_error_errno(r, "Failed to set up job watcher: %m"); + + r = dispatch_unit_method(bus, w, restart_units, "RestartUnit"); + if (r < 0) + return r; + + r = dispatch_unit_method(bus, w, reload_or_restart_units, "ReloadOrRestartUnit"); + if (r < 0) + return r; + + return 0; +} + static int append_overlayfs_path_option( char **options, const char *separator, @@ -1673,15 +1819,15 @@ static int unmerge( char **hierarchies, bool no_reload) { + _cleanup_set_free_ Set *units_to_restart = NULL, *units_to_reload_or_restart = NULL; bool need_to_reload; int r; (void) DLOPEN_LIBMOUNT(LOG_DEBUG, SD_ELF_NOTE_DLOPEN_PRIORITY_REQUIRED); - r = need_reload(image_class, hierarchies, no_reload); + r = get_extension_release_metadata(image_class, hierarchies, no_reload, &need_to_reload, &units_to_restart, &units_to_reload_or_restart); if (r < 0) return r; - need_to_reload = r > 0; r = pidref_safe_fork( "(sd-unmerge)", @@ -1706,6 +1852,11 @@ static int unmerge( return r; } + /* The sets will be empty when opted out */ + r = restart_units(units_to_restart, units_to_reload_or_restart); + if (r < 0) + return r; + return 0; } @@ -2293,15 +2444,22 @@ static int merge(ImageClass image_class, if (r > 0) return log_error_errno(SYNTHETIC_ERRNO(EPROTO), "Failed to merge hierarchies"); - r = need_reload(image_class, hierarchies, no_reload); + _cleanup_set_free_ Set *units_to_restart = NULL, *units_to_reload_or_restart = NULL; + bool need_to_reload; + r = get_extension_release_metadata(image_class, hierarchies, no_reload, &need_to_reload, &units_to_restart, &units_to_reload_or_restart); if (r < 0) return r; - if (r > 0) { + if (need_to_reload) { r = daemon_reload(); if (r < 0) return r; } + /* The sets will be empty when opted out */ + r = restart_units(units_to_restart, units_to_reload_or_restart); + if (r < 0) + return r; + return 1; } diff --git a/test/units/TEST-50-DISSECT.sysext.sh b/test/units/TEST-50-DISSECT.sysext.sh index 60fb216ece..fd62dcfe81 100755 --- a/test/units/TEST-50-DISSECT.sysext.sh +++ b/test/units/TEST-50-DISSECT.sysext.sh @@ -1675,6 +1675,194 @@ run_systemd_sysext "$fake_root" unmerge extension_verify_after_unmerge "$fake_root" "$hierarchy" -h ) +( init_trap +: "Check EXTENSION_RESTART_UNITS= (re)starts units after merge and stops vanished ones on unmerge" +# Talks to the real service manager, skip the --root= variant (it is a no-op there) +if [ "$roots_dir" != "" ]; then + exit 0 +fi + +ext_name="test-restart-extension" +ext_dir="/var/lib/extensions/$ext_name" +host_unit="test-restart-host.service" +host_unit_file="/run/systemd/system/$host_unit" +ext_unit="test-restart-ext.service" + +cat >"$host_unit_file" </dev/null || :" +systemctl daemon-reload +systemctl start "$host_unit" +host_pid_before=$(systemctl show -P MainPID "$host_unit") + +mkdir -p "$ext_dir/usr/lib/extension-release.d" "$ext_dir/usr/lib/systemd/system" +prepend_trap "rm -rf ${ext_dir@Q}" +cat >"$ext_dir/usr/lib/extension-release.d/extension-release.$ext_name" <"$ext_dir/usr/lib/systemd/system/$ext_unit" <&2 "Unexpected restart of host unit" + exit 1 +fi +if systemctl --quiet is-active "$ext_unit"; then + echo >&2 "Unexpected start of extension unit" + exit 1 +fi +systemd-sysext unmerge --no-reload + +# merge: host unit is restarted (new PID), extension unit is started +systemd-sysext merge +host_pid_merged=$(systemctl show -P MainPID "$host_unit") +if [ "$host_pid_before" = "$host_pid_merged" ]; then + echo >&2 "Missing restart of host unit" + exit 1 +fi +if ! systemctl --quiet is-active "$ext_unit"; then + echo >&2 "Missing start of extension unit" + exit 1 +fi +host_pid_before_refresh="$host_pid_merged" +systemd-sysext refresh --always-refresh=yes +host_pid_merged=$(systemctl show -P MainPID "$host_unit") +if [ "$host_pid_before_refresh" = "$host_pid_merged" ]; then + echo >&2 "Missing restart of host unit after refresh" + exit 1 +fi +if ! systemctl --quiet is-active "$ext_unit"; then + echo >&2 "Missing start of extension unit" + exit 1 +fi + +# unmerge: host unit file is still in /run -> restart again (new PID) +# but extension unit file is gone -> fallback to StopUnit +systemd-sysext unmerge +host_pid_unmerged=$(systemctl show -P MainPID "$host_unit") +if [ "$host_pid_merged" = "$host_pid_unmerged" ]; then + echo >&2 "Missing restart of host unit" + exit 1 +fi +if systemctl --quiet is-active "$ext_unit"; then + echo >&2 "Missing stop of extension unit" + exit 1 +fi +) + +( init_trap +: "Check EXTENSION_RELOAD_OR_RESTART_UNITS= reloads units after merge and stops vanished ones on unmerge" +# Talks to the real service manager, skip the --root= variant (it is a no-op there) +if [ "$roots_dir" != "" ]; then + exit 0 +fi + +ext_name="test-reload-or-restart-extension" +ext_dir="/var/lib/extensions/$ext_name" +host_unit="test-reload-or-restart-host.service" +host_unit_file="/run/systemd/system/$host_unit" +ext_unit="test-reload-or-restart-ext.service" +host_stamp="/run/test-reload-or-restart-host.stamp" +ext_stamp="/run/test-reload-or-restart-ext.stamp" + +cat >"$host_unit_file" </dev/null || :" +systemctl daemon-reload +systemctl start "$host_unit" +host_pid_before=$(systemctl show -P MainPID "$host_unit") + +mkdir -p "$ext_dir/usr/lib/extension-release.d" "$ext_dir/usr/lib/systemd/system" +prepend_trap "rm -rf ${ext_dir@Q} ${ext_stamp@Q}" +cat >"$ext_dir/usr/lib/extension-release.d/extension-release.$ext_name" <"$ext_dir/usr/lib/systemd/system/$ext_unit" < reloaded (same PID, stamp file created), +# extension unit is started (no reload because it was not running yet) +rm -f "$host_stamp" "$ext_stamp" +systemd-sysext merge +host_pid_merged=$(systemctl show -P MainPID "$host_unit") +if [ "$host_pid_before" != "$host_pid_merged" ]; then + echo >&2 "Unexpected restart of host unit (should have reloaded)" + exit 1 +fi +if [ ! -e "$host_stamp" ]; then + echo >&2 "Host unit was not reloaded on merge (stamp missing)" + exit 1 +fi +if ! systemctl --quiet is-active "$ext_unit"; then + echo >&2 "Missing start of extension unit" + exit 1 +fi +ext_pid_merged=$(systemctl show -P MainPID "$ext_unit") + +# refresh: both units already running and reload-capable -> reloaded, MainPIDs unchanged, +# both stamp files re-created +rm -f "$host_stamp" "$ext_stamp" +systemd-sysext refresh --always-refresh=yes +host_pid_refreshed=$(systemctl show -P MainPID "$host_unit") +if [ "$host_pid_merged" != "$host_pid_refreshed" ]; then + echo >&2 "Unexpected restart of host unit on refresh (should have reloaded)" + exit 1 +fi +if [ ! -e "$host_stamp" ]; then + echo >&2 "Host unit was not reloaded on refresh (stamp missing)" + exit 1 +fi +ext_pid_refreshed=$(systemctl show -P MainPID "$ext_unit") +if [ "$ext_pid_merged" != "$ext_pid_refreshed" ]; then + echo >&2 "Unexpected restart of extension unit on refresh (should have reloaded)" + exit 1 +fi +if [ ! -e "$ext_stamp" ]; then + echo >&2 "Extension unit was not reloaded on refresh (stamp missing)" + exit 1 +fi + +# unmerge: host unit file still in /run -> reloaded again (same PID, stamp re-created) +# but extension unit file is gone -> fallback to StopUnit +rm -f "$host_stamp" "$ext_stamp" +systemd-sysext unmerge +host_pid_unmerged=$(systemctl show -P MainPID "$host_unit") +if [ "$host_pid_refreshed" != "$host_pid_unmerged" ]; then + echo >&2 "Unexpected restart of host unit on unmerge (should have reloaded)" + exit 1 +fi +if [ ! -e "$host_stamp" ]; then + echo >&2 "Host unit was not reloaded on unmerge (stamp missing)" + exit 1 +fi +if systemctl --quiet is-active "$ext_unit"; then + echo >&2 "Missing stop of extension unit" + exit 1 +fi +) + } # End of run_sysext_tests