From b1ed7e6749541f03c2a35f05a91c5d950908e71f Mon Sep 17 00:00:00 2001
From: Mike Yuan <me@yhndnzj.com>
Date: Tue, 11 Jun 2024 16:00:22 +0200
Subject: [PATCH] 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.
---
 src/home/homework.c        | 22 +++++++++++-----------
 src/shared/bus-unit-util.c | 19 -------------------
 src/shared/bus-unit-util.h |  2 --
 src/sleep/sleep.c          | 10 +++++++---
 4 files changed, 18 insertions(+), 35 deletions(-)

diff --git a/src/home/homework.c b/src/home/homework.c
index 2537e8d1ddd..00e74894b32 100644
--- a/src/home/homework.c
+++ b/src/home/homework.c
@@ -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)
diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c
index 751cb29c624..259fbeaf5c4 100644
--- a/src/shared/bus-unit-util.c
+++ b/src/shared/bus-unit-util.c
@@ -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;
-}
diff --git a/src/shared/bus-unit-util.h b/src/shared/bus-unit-util.h
index ea4056c6910..6f0d55971ce 100644
--- a/src/shared/bus-unit-util.h
+++ b/src/shared/bus-unit-util.h
@@ -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);
diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c
index 7e5075a63cb..15ef7ae9131 100644
--- a/src/sleep/sleep.c
+++ b/src/sleep/sleep.c
@@ -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.");
-- 
2.47.3