diff --git a/src/home/homework.c b/src/home/homework.c index e10922cefd..428f832911 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -1866,10 +1866,13 @@ 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."); @@ -1878,7 +1881,8 @@ static int user_session_freezer(uid_t uid, bool freeze_now, UnitFreezer *ret) { 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) {}; + + *ret = NULL; return 0; } @@ -1891,12 +1895,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 +1916,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 +1939,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 +1960,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..8de9f374e3 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; } +typedef struct UnitFreezer { + char *name; + sd_bus *bus; +} UnitFreezer; + /* Wait for 1.5 seconds at maximum for freeze operation */ #define FREEZE_BUS_CALL_TIMEOUT (1500 * USEC_PER_MSEC) -int unit_freezer_new(const char *name, UnitFreezer *ret) { - _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; - _cleanup_free_ char *namedup = NULL; +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..1911ef256e 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,13 +603,9 @@ 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 + 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 is set to false). This is not recommended, " "and might result in unexpected behavior, particularly in suspend-then-hibernate " @@ -641,6 +637,9 @@ static int run(int argc, char *argv[]) { } + if (user_slice_freezer) + RET_GATHER(r, unit_freezer_thaw(user_slice_freezer)); + return r; }