From: Adrian Vovk <adrianvovk@gmail.com>
Date: Sat, 23 Dec 2023 23:00:48 +0000 (-0500)
Subject: homework: Lock/Unlock: Freeze/Thaw user session
X-Git-Tag: v256-rc1~617^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a5b009d9355595b30c4da6f4499dd05f6e23ba8c;p=thirdparty%2Fsystemd.git

homework: Lock/Unlock: Freeze/Thaw user session

Whenever a home directory is in a locked state, accessing the files of
the home directory is extremely likely to cause the thread to hang. This
will put the session in a strange state, where some threads are hanging
due to file access and others are not hanging because they are not
trying to access any of the user's files.

This can lead to a whole slew of consequences. For example, imagine a
likely situation where the Wayland compositor is not hanging, but the
user's open apps are. Eventually, the compositor will detect that none
of the apps are responding to its pings, assume that they're frozen
(which they are), and kill them. The systemd user instance can end up in
a similarly confused state and start killing user services. In the worst
case, killing an app at an unexpected moment can lead to data loss.

The solution is to suspend execution of the whole user session by
freezing the user's slice.
---

diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md
index dc6a7b4b17f..d2fb74dd96d 100644
--- a/docs/ENVIRONMENT.md
+++ b/docs/ENVIRONMENT.md
@@ -557,6 +557,14 @@ SYSTEMD_HOME_DEBUG_SUFFIX=foo \
   `mkfs` when formatting LUKS home directories. There's one variable for each
   of the supported file systems for the LUKS home directory backend.
 
+* `$SYSTEMD_HOME_LOCK_FREEZE_SESSION` - Takes a boolean. When false, the user's
+  session will not be frozen when the home directory is locked. Note that the kernel
+  may still freeze any task that tries to access data from the user's locked home
+  directory. This can lead to data loss, security leaks, or other undesired behavior
+  caused by parts of the session becoming unresponsive due to disk I/O while other
+  parts of the session continue running. Thus, we highly recommend that this variable
+  isn't used unless necessary. Defaults to true.
+
 `kernel-install`:
 
 * `$KERNEL_INSTALL_BYPASS` – If set to "1", execution of kernel-install is skipped
diff --git a/src/home/homework.c b/src/home/homework.c
index 39e00514865..531443e7576 100644
--- a/src/home/homework.c
+++ b/src/home/homework.c
@@ -4,11 +4,14 @@
 #include <sys/mount.h>
 
 #include "blockdev-util.h"
+#include "bus-unit-util.h"
 #include "chown-recursive.h"
 #include "copy.h"
+#include "env-util.h"
 #include "fd-util.h"
 #include "fileio.h"
 #include "filesystems.h"
+#include "format-util.h"
 #include "fs-util.h"
 #include "home-util.h"
 #include "homework-blob.h"
@@ -1820,8 +1823,37 @@ static int home_inspect(UserRecord *h, UserRecord **ret_home) {
         return 1;
 }
 
+static int user_session_freezer(uid_t uid, bool freeze_now, UnitFreezer *ret) {
+        _cleanup_free_ char *unit = NULL;
+        int r;
+
+        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.");
+        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) {};
+                return 0;
+        }
+
+        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);
+        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);
@@ -1837,16 +1869,27 @@ 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.");
+
         r = home_lock_luks(h, &setup);
         if (r < 0)
                 return r;
 
+        unit_freezer_done(&freezer); /* Don't thaw the user session. */
+
         log_info("Everything completed.");
         return 1;
 }
 
 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;
 
@@ -1868,6 +1911,11 @@ static int home_unlock(UserRecord *h) {
         if (r < 0)
                 return r;
 
+        /* We want to thaw the session only after it's safe to access $HOME */
+        r = user_session_freezer(h->uid, /* freeze_now= */ false, &freezer);
+        if (r < 0)
+                log_warning_errno(r, "Failed to recover freezer for user session, ignoring: %m");
+
         log_info("Everything completed.");
         return 1;
 }