diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 2aa4336121..2515f54ada 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -834,11 +834,13 @@ static int method_clean_unit(sd_bus_message *message, void *userdata, sd_bus_err } static int method_freeze_unit(sd_bus_message *message, void *userdata, sd_bus_error *error) { - return method_generic_unit_operation(message, userdata, error, bus_unit_method_freeze, 0); + /* Only active units can be frozen, which must be properly loaded already */ + return method_generic_unit_operation(message, userdata, error, bus_unit_method_freeze, GENERIC_UNIT_VALIDATE_LOADED); } static int method_thaw_unit(sd_bus_message *message, void *userdata, sd_bus_error *error) { - return method_generic_unit_operation(message, userdata, error, bus_unit_method_thaw, 0); + /* Same as freeze above */ + return method_generic_unit_operation(message, userdata, error, bus_unit_method_thaw, GENERIC_UNIT_VALIDATE_LOADED); } static int method_reset_failed_unit(sd_bus_message *message, void *userdata, sd_bus_error *error) { diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 20fa43af9e..953cd512a3 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -723,15 +723,13 @@ int bus_unit_method_clean(sd_bus_message *message, void *userdata, sd_bus_error } static int bus_unit_method_freezer_generic(sd_bus_message *message, void *userdata, sd_bus_error *error, FreezerAction action) { - const char* perm; Unit *u = ASSERT_PTR(userdata); - bool reply_no_delay = false; int r; assert(message); assert(IN_SET(action, FREEZER_FREEZE, FREEZER_THAW)); - perm = action == FREEZER_FREEZE ? "stop" : "start"; + const char *perm = action == FREEZER_FREEZE ? "stop" : "start"; r = mac_selinux_unit_access_check(u, message, perm, error); if (r < 0) @@ -750,19 +748,19 @@ static int bus_unit_method_freezer_generic(sd_bus_message *message, void *userda r = unit_freezer_action(u, action); if (r == -EOPNOTSUPP) - return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Unit does not support freeze/thaw."); + return sd_bus_error_set(error, SD_BUS_ERROR_NOT_SUPPORTED, "Unit does not support freeze/thaw"); if (r == -EBUSY) - return sd_bus_error_set(error, BUS_ERROR_UNIT_BUSY, "Unit has a pending job."); + return sd_bus_error_set(error, BUS_ERROR_UNIT_BUSY, "Unit has a pending job"); if (r == -EHOSTDOWN) - return sd_bus_error_set(error, BUS_ERROR_UNIT_INACTIVE, "Unit is inactive."); + return sd_bus_error_set(error, BUS_ERROR_UNIT_INACTIVE, "Unit is not active"); if (r == -EALREADY) - return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Previously requested freezer operation for unit is still in progress."); + return sd_bus_error_set(error, BUS_ERROR_UNIT_BUSY, "Previously requested freezer operation for unit is still in progress"); if (r == -ECHILD) - return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Unit is frozen by a parent slice."); + return sd_bus_error_set(error, SD_BUS_ERROR_FAILED, "Unit is frozen by a parent slice"); if (r < 0) return r; - if (r == 0) - reply_no_delay = true; + + bool reply_now = r == 0; if (u->pending_freezer_invocation) { bus_unit_send_pending_freezer_message(u, true); @@ -771,7 +769,7 @@ static int bus_unit_method_freezer_generic(sd_bus_message *message, void *userda u->pending_freezer_invocation = sd_bus_message_ref(message); - if (reply_no_delay) { + if (reply_now) { r = bus_unit_send_pending_freezer_message(u, false); if (r < 0) return r; diff --git a/src/home/homework.c b/src/home/homework.c index e10922cefd..b556904651 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -1866,19 +1866,22 @@ static int home_inspect(UserRecord *h, UserRecord **ret_home) { return 1; } -static int user_session_freezer(uid_t uid, bool freeze_now, UnitFreezer *ret) { +static int user_session_freezer(uid_t uid, bool freeze_now, UnitFreezer **ret) { _cleanup_free_ char *unit = NULL; int r; + assert(uid_is_valid(uid)); + assert(ret); + r = getenv_bool("SYSTEMD_HOME_LOCK_FREEZE_SESSION"); if (r < 0 && r != -ENXIO) - log_warning_errno(r, "Cannot parse value of $SYSTEMD_HOME_LOCK_FREEZE_SESSION, ignoring."); + log_warning_errno(r, "Cannot parse value of $SYSTEMD_HOME_LOCK_FREEZE_SESSION, ignoring: %m"); else if (r == 0) { if (freeze_now) - log_notice("Session remains unfrozen on explicit request ($SYSTEMD_HOME_LOCK_FREEZE_SESSION " - "is set to false). This is not recommended, and might result in unexpected behavior " - "including data loss!"); - *ret = (UnitFreezer) {}; + log_notice("Session remains unfrozen on explicit request ($SYSTEMD_HOME_LOCK_FREEZE_SESSION=0).\n" + "This is not recommended, and might result in unexpected behavior including data loss!"); + + *ret = NULL; return 0; } @@ -1891,12 +1894,12 @@ static int user_session_freezer(uid_t uid, bool freeze_now, UnitFreezer *ret) { r = unit_freezer_new(unit, ret); if (r < 0) return r; + return 1; } static int home_lock(UserRecord *h) { _cleanup_(home_setup_done) HomeSetup setup = HOME_SETUP_INIT; - _cleanup_(unit_freezer_done_thaw) UnitFreezer freezer = {}; int r; assert(h); @@ -1912,19 +1915,19 @@ static int home_lock(UserRecord *h) { if (r != USER_TEST_MOUNTED) return log_error_errno(SYNTHETIC_ERRNO(ENOEXEC), "Home directory of %s is not mounted, can't lock.", h->user_name); - r = user_session_freezer(h->uid, /* freeze_now= */ true, &freezer); - if (r < 0) - log_warning_errno(r, "Failed to freeze user session, ignoring: %m"); - else if (r == 0) - log_info("User session freeze disabled, skipping."); - else - log_info("Froze user session."); + _cleanup_(unit_freezer_freep) UnitFreezer *f = NULL; - r = home_lock_luks(h, &setup); + r = user_session_freezer(h->uid, /* freeze_now= */ true, &f); if (r < 0) return r; - unit_freezer_done(&freezer); /* Don't thaw the user session. */ + r = home_lock_luks(h, &setup); + if (r < 0) { + if (f) + (void) unit_freezer_thaw(f); + + return r; + } /* Explicitly flush any per-user key from the keyring */ (void) keyring_flush(h); @@ -1935,7 +1938,6 @@ static int home_lock(UserRecord *h) { static int home_unlock(UserRecord *h) { _cleanup_(home_setup_done) HomeSetup setup = HOME_SETUP_INIT; - _cleanup_(unit_freezer_done_thaw) UnitFreezer freezer = {}; _cleanup_(password_cache_free) PasswordCache cache = {}; int r; @@ -1957,10 +1959,14 @@ static int home_unlock(UserRecord *h) { if (r < 0) return r; + _cleanup_(unit_freezer_freep) UnitFreezer *f = NULL; + /* We want to thaw the session only after it's safe to access $HOME */ - r = user_session_freezer(h->uid, /* freeze_now= */ false, &freezer); + r = user_session_freezer(h->uid, /* freeze_now= */ false, &f); + if (r > 0) + r = unit_freezer_thaw(f); if (r < 0) - log_warning_errno(r, "Failed to recover freezer for user session, ignoring: %m"); + return r; log_info("Everything completed."); return 1; diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index fd46332821..22dbf62ab2 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -2,6 +2,7 @@ #include "af-list.h" #include "alloc-util.h" +#include "bus-common-errors.h" #include "bus-error.h" #include "bus-locator.h" #include "bus-unit-util.h" @@ -2939,41 +2940,51 @@ int bus_service_manager_reload(sd_bus *bus) { return 0; } -/* Wait for 1.5 seconds at maximum for freeze operation */ -#define FREEZE_BUS_CALL_TIMEOUT (1500 * USEC_PER_MSEC) +typedef struct UnitFreezer { + char *name; + sd_bus *bus; +} UnitFreezer; -int unit_freezer_new(const char *name, UnitFreezer *ret) { - _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; - _cleanup_free_ char *namedup = NULL; +/* Wait for 10 seconds at maximum for freezer operation */ +#define FREEZE_BUS_CALL_TIMEOUT (10 * USEC_PER_SEC) + +UnitFreezer* unit_freezer_free(UnitFreezer *f) { + if (!f) + return NULL; + + free(f->name); + sd_bus_flush_close_unref(f->bus); + + return mfree(f); +} + +int unit_freezer_new(const char *name, UnitFreezer **ret) { + _cleanup_(unit_freezer_freep) UnitFreezer *f = NULL; int r; assert(name); assert(ret); - namedup = strdup(name); - if (!namedup) - return log_oom_debug(); + f = new(UnitFreezer, 1); + if (!f) + return log_oom(); - r = bus_connect_system_systemd(&bus); - if (r < 0) - return log_debug_errno(r, "Failed to open connection to systemd: %m"); - - (void) sd_bus_set_method_call_timeout(bus, FREEZE_BUS_CALL_TIMEOUT); - - *ret = (UnitFreezer) { - .name = TAKE_PTR(namedup), - .bus = TAKE_PTR(bus), + *f = (UnitFreezer) { + .name = strdup(name), }; + if (!f->name) + return log_oom(); + + r = bus_connect_system_systemd(&f->bus); + if (r < 0) + return log_error_errno(r, "Failed to open connection to systemd: %m"); + + (void) sd_bus_set_method_call_timeout(f->bus, FREEZE_BUS_CALL_TIMEOUT); + + *ret = TAKE_PTR(f); return 0; } -void unit_freezer_done(UnitFreezer *f) { - assert(f); - - f->name = mfree(f->name); - f->bus = sd_bus_flush_close_unref(f->bus); -} - static int unit_freezer_action(UnitFreezer *f, bool freeze) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; int r; @@ -2988,11 +2999,22 @@ static int unit_freezer_action(UnitFreezer *f, bool freeze) { /* reply = */ NULL, "s", f->name); - if (r < 0) - return log_debug_errno(r, "Failed to %s unit %s: %s", - freeze ? "freeze" : "thaw", f->name, bus_error_message(&error, r)); + if (r < 0) { + if (sd_bus_error_has_names(&error, + BUS_ERROR_NO_SUCH_UNIT, + BUS_ERROR_UNIT_INACTIVE, + SD_BUS_ERROR_NOT_SUPPORTED)) { - return 0; + log_debug_errno(r, "Skipping freezer for '%s': %s", f->name, bus_error_message(&error, r)); + return 0; + } + + return log_error_errno(r, "Failed to %s unit '%s': %s", + freeze ? "freeze" : "thaw", f->name, bus_error_message(&error, r)); + } + + log_info("Successfully %s unit '%s'.", freeze ? "froze" : "thawed", f->name); + return 1; } int unit_freezer_freeze(UnitFreezer *f) { @@ -3003,8 +3025,8 @@ int unit_freezer_thaw(UnitFreezer *f) { return unit_freezer_action(f, false); } -int unit_freezer_new_freeze(const char *name, UnitFreezer *ret) { - _cleanup_(unit_freezer_done) UnitFreezer f = {}; +int unit_freezer_new_freeze(const char *name, UnitFreezer **ret) { + _cleanup_(unit_freezer_freep) UnitFreezer *f = NULL; int r; assert(name); @@ -3014,20 +3036,10 @@ int unit_freezer_new_freeze(const char *name, UnitFreezer *ret) { if (r < 0) return r; - r = unit_freezer_freeze(&f); + r = unit_freezer_freeze(f); if (r < 0) return r; - *ret = TAKE_STRUCT(f); + *ret = TAKE_PTR(f); return 0; } - -void unit_freezer_done_thaw(UnitFreezer *f) { - assert(f); - - if (!f->name) - return; - - (void) unit_freezer_thaw(f); - unit_freezer_done(f); -} diff --git a/src/shared/bus-unit-util.h b/src/shared/bus-unit-util.h index e4168a4425..ea4056c691 100644 --- a/src/shared/bus-unit-util.h +++ b/src/shared/bus-unit-util.h @@ -36,17 +36,14 @@ int unit_info_compare(const UnitInfo *a, const UnitInfo *b); int bus_service_manager_reload(sd_bus *bus); -typedef struct UnitFreezer { - char *name; - sd_bus *bus; -} UnitFreezer; +typedef struct UnitFreezer UnitFreezer; -int unit_freezer_new(const char *name, UnitFreezer *ret); -void unit_freezer_done(UnitFreezer *f); +UnitFreezer* unit_freezer_free(UnitFreezer *f); +DEFINE_TRIVIAL_CLEANUP_FUNC(UnitFreezer*, unit_freezer_free); + +int unit_freezer_new(const char *name, UnitFreezer **ret); int unit_freezer_freeze(UnitFreezer *f); int unit_freezer_thaw(UnitFreezer *f); -int unit_freezer_new_freeze(const char *name, UnitFreezer *ret); - -void unit_freezer_done_thaw(UnitFreezer *f); +int unit_freezer_new_freeze(const char *name, UnitFreezer **ret); diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index c96207428d..0402bb07f3 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -580,7 +580,7 @@ static int parse_argv(int argc, char *argv[]) { } static int run(int argc, char *argv[]) { - _cleanup_(unit_freezer_done_thaw) UnitFreezer user_slice_freezer = {}; + _cleanup_(unit_freezer_freep) UnitFreezer *user_slice_freezer = NULL; _cleanup_(sleep_config_freep) SleepConfig *sleep_config = NULL; int r; @@ -603,17 +603,12 @@ static int run(int argc, char *argv[]) { r = getenv_bool("SYSTEMD_SLEEP_FREEZE_USER_SESSIONS"); if (r < 0 && r != -ENXIO) log_warning_errno(r, "Cannot parse value of $SYSTEMD_SLEEP_FREEZE_USER_SESSIONS, ignoring."); - if (r != 0) { - r = unit_freezer_new_freeze(SPECIAL_USER_SLICE, &user_slice_freezer); - if (r < 0) - log_warning_errno(r, "Failed to freeze user sessions, ignoring: %m"); - else - log_info("Froze user sessions"); - } else - log_notice("User sessions remain unfrozen on explicit request " - "($SYSTEMD_SLEEP_FREEZE_USER_SESSIONS is set to false). This is not recommended, " - "and might result in unexpected behavior, particularly in suspend-then-hibernate " - "operations or setups with encrypted home directories."); + if (r != 0) + (void) unit_freezer_new_freeze(SPECIAL_USER_SLICE, &user_slice_freezer); + else + log_notice("User sessions remain unfrozen on explicit request ($SYSTEMD_SLEEP_FREEZE_USER_SESSIONS=0).\n" + "This is not recommended, and might result in unexpected behavior, particularly\n" + "in suspend-then-hibernate operations or setups with encrypted home directories."); switch (arg_operation) { @@ -641,6 +636,9 @@ static int run(int argc, char *argv[]) { } + if (user_slice_freezer) + RET_GATHER(r, unit_freezer_thaw(user_slice_freezer)); + return r; }