sleep,home: always initialize UnitFreezer if used

Previously, unit_freezer_new_freeze() would only return
UnitFreezer object if FreezeUnit() succeeds. This is not
ideal though, as a failed bus call doesn't mean the action
actually failed. E.g. a timeout might occur because pid1
is waiting for cgroup event from kernel, while the bus call
timeout was exceeded (#33269). In such a case, ThawUnit()
will never be called, resulting in frozen units remain that
way after resuming from sleep.

Therefore, let's get rid of unit_freezer_new_freeze(),
and make sure as long as unit freezer is involved, we'll
call ThawUnit() when we're done. This should make things
a lot more robust.
This commit is contained in:
Mike Yuan
2024-06-11 16:00:22 +02:00
parent 1b5caddfee
commit b1ed7e6749
4 changed files with 18 additions and 35 deletions

View File

@@ -1870,7 +1870,7 @@ 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_new(uid_t uid, UnitFreezer **ret) {
_cleanup_free_ char *unit = NULL;
int r;
@@ -1881,10 +1881,6 @@ static int user_session_freezer(uid_t uid, bool freeze_now, UnitFreezer **ret) {
if (r < 0 && r != -ENXIO)
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=0).\n"
"This is not recommended, and might result in unexpected behavior including data loss!");
*ret = NULL;
return 0;
}
@@ -1892,10 +1888,7 @@ static int user_session_freezer(uid_t uid, bool freeze_now, UnitFreezer **ret) {
if (asprintf(&unit, "user-" UID_FMT ".slice", uid) < 0)
return log_oom();
if (freeze_now)
r = unit_freezer_new_freeze(unit, ret);
else
r = unit_freezer_new(unit, ret);
r = unit_freezer_new(unit, ret);
if (r < 0)
return r;
@@ -1921,9 +1914,16 @@ static int home_lock(UserRecord *h) {
_cleanup_(unit_freezer_freep) UnitFreezer *f = NULL;
r = user_session_freezer(h->uid, /* freeze_now= */ true, &f);
r = user_session_freezer_new(h->uid, &f);
if (r < 0)
return r;
if (r > 0) {
r = unit_freezer_freeze(f);
if (r < 0)
return r;
} else
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!");
r = home_lock_luks(h, &setup);
if (r < 0) {
@@ -1966,7 +1966,7 @@ static int home_unlock(UserRecord *h) {
_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, &f);
r = user_session_freezer_new(h->uid, &f);
if (r > 0)
r = unit_freezer_thaw(f);
if (r < 0)

View File

@@ -3025,22 +3025,3 @@ int unit_freezer_freeze(UnitFreezer *f) {
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_freep) UnitFreezer *f = NULL;
int r;
assert(name);
assert(ret);
r = unit_freezer_new(name, &f);
if (r < 0)
return r;
r = unit_freezer_freeze(f);
if (r < 0)
return r;
*ret = TAKE_PTR(f);
return 0;
}

View File

@@ -45,5 +45,3 @@ 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);

View File

@@ -604,9 +604,13 @@ 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: %m");
if (r != 0)
(void) unit_freezer_new_freeze(SPECIAL_USER_SLICE, &user_slice_freezer);
else
if (r != 0) {
r = unit_freezer_new(SPECIAL_USER_SLICE, &user_slice_freezer);
if (r < 0)
return r;
(void) unit_freezer_freeze(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.");