Merge pull request #30612 from AdrianVovk/sleep-freeze-user-seesions

Freeze user sessions for all types of sleep

NEWS

@@ -27,6 +27,15 @@ CHANGES WITH 256 in spe:
           mounted at some path, for example /boot/efi/ (this type of setup is
           obsolete but is still commonly found).
 
+        * The behavior of systemd-sleep and systemd-homed has been updated to
+          freeze user sessions when entering the various sleep modes or when
+          locking a homed-managed home area. This is known to cause issues with
+          the proprietary NVIDIA drivers. Packagers of the NVIDIA proprietary
+          drivers may want to add drop-in configuration files that set
+          SYSTEMD_SLEEP_FREEZE_USER_SESSION=false for systemd-suspend.service
+          and related services, and SYSTEMD_HOME_LOCK_FREEZE_SESSION=false for
+          systemd-homed.service.
+
         Network Management:
 
         * systemd-networkd's proxy support gained a new option to configure

docs/ENVIRONMENT.md

@@ -558,6 +558,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
@@ -631,6 +639,14 @@ SYSTEMD_HOME_DEBUG_SUFFIX=foo \
   file (containing firmware measurement data) to read. This allows overriding
   the default of `/sys/kernel/security/tpm0/binary_bios_measurements`.
 
+`systemd-sleep`:
+
+* `$SYSTEMD_SLEEP_FREEZE_USER_SESSIONS` - Takes a boolean. When true (the default),
+  `user.slice` will be frozen during sleep. When false it will not be. We recommend
+  against using this variable, because it can lead to undesired behavior, especially
+  for systems that use home directory encryption and for
+  `systemd-suspend-then-hibernate.service`.
+
 Tools using the Varlink protocol (such as `varlinkctl`) or sd-bus (such as
 `busctl`):
 

man/systemd-suspend.service.xml

@@ -66,7 +66,9 @@
     same executables are run, but the first argument is now
     <literal>post</literal>. All executables in this directory are
     executed in parallel, and execution of the action is not continued
-    until all executables have finished.</para>
+    until all executables have finished. Note that <filename>user.slice</filename> will
+    be frozen while the executables are running, so they should not attempt to
+    communicate with any user services expecting a reply.</para>
 
     <para>Note that scripts or binaries dropped in
     <filename>/usr/lib/systemd/system-sleep/</filename> are intended
@@ -90,6 +92,11 @@
     <filename>sleep.conf.d</filename> file. See
     <citerefentry><refentrytitle>systemd-sleep.conf</refentrytitle><manvolnum>5</manvolnum></citerefentry>.
     </para>
+
+    <para>Note that by default these services freeze <filename>user.slice</filename> while they run. This prevents
+    the execution of any process in any of the user sessions while the system is entering into and resuming from
+    sleep. Thus, this prevents the hooks in <filename>/usr/lib/systemd/system-sleep/</filename>, or any other process
+    for that matter, from communicating with any user session process during sleep.</para>
   </refsect1>
 
   <refsect1>

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;
 }

src/shared/bus-unit-util.c

@@ -10,6 +10,7 @@
 #include "cgroup-setup.h"
 #include "cgroup-util.h"
 #include "condition.h"
+#include "constants.h"
 #include "coredump-util.h"
 #include "cpu-set-util.h"
 #include "dissect-image.h"
@@ -2937,3 +2938,89 @@ int bus_service_manager_reload(sd_bus *bus) {
 
         return 0;
 }
+
+int unit_freezer_new(const char *name, UnitFreezer *ret) {
+        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
+        _cleanup_free_ char *namedup = NULL;
+        int r;
+
+        assert(name);
+        assert(ret);
+
+        namedup = strdup(name);
+        if (!namedup)
+                return log_oom_debug();
+
+        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_TIMEOUT);
+
+        *ret = (UnitFreezer) {
+                .name = TAKE_PTR(namedup),
+                .bus = TAKE_PTR(bus),
+        };
+        return 0;
+}
+
+static int unit_freezer_action(bool freeze, UnitFreezer *f) {
+        _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
+        int r;
+
+        assert(f);
+        assert(f->bus);
+        assert(f->name);
+
+        r = bus_call_method(f->bus, bus_systemd_mgr, freeze ? "FreezeUnit" : "ThawUnit",
+                            &error, 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));
+
+        return 0;
+}
+
+int unit_freezer_freeze(UnitFreezer *f) {
+        return unit_freezer_action(true, f);
+}
+
+int unit_freezer_thaw(UnitFreezer *f) {
+        return unit_freezer_action(false, f);
+}
+
+void unit_freezer_done(UnitFreezer *f) {
+        assert(f);
+
+        f->name = mfree(f->name);
+        f->bus = sd_bus_flush_close_unref(f->bus);
+}
+
+int unit_freezer_new_freeze(const char *name, UnitFreezer *ret) {
+        _cleanup_(unit_freezer_done) UnitFreezer f = {};
+        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_STRUCT(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);
+}

src/shared/bus-unit-util.h

@@ -35,3 +35,20 @@ int unit_load_state(sd_bus *bus, const char *name, char **load_state);
 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;
+
+int unit_freezer_new(const char *name, UnitFreezer *ret);
+
+int unit_freezer_freeze(UnitFreezer *freezer);
+
+int unit_freezer_thaw(UnitFreezer *freezer);
+
+void unit_freezer_done(UnitFreezer *freezer);
+
+int unit_freezer_new_freeze(const char *name, UnitFreezer *ret);
+
+void unit_freezer_done_thaw(UnitFreezer *freezer);

src/sleep/sleep.c

@@ -23,10 +23,12 @@
 #include "build.h"
 #include "bus-error.h"
 #include "bus-locator.h"
+#include "bus-unit-util.h"
 #include "bus-util.h"
 #include "constants.h"
 #include "devnum-util.h"
 #include "efivars.h"
+#include "env-util.h"
 #include "exec-util.h"
 #include "fd-util.h"
 #include "fileio.h"
@@ -444,38 +446,11 @@ static int custom_timer_suspend(const SleepConfig *sleep_config) {
         return 1;
 }
 
-/* Freeze when invoked and thaw on cleanup */
-static int freeze_thaw_user_slice(const char **method) {
-        _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
-        _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL;
-        int r;
-
-        if (!method || !*method)
-                return 0;
-
-        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_TIMEOUT);
-
-        r = bus_call_method(bus, bus_systemd_mgr, *method, &error, NULL, "s", SPECIAL_USER_SLICE);
-        if (r < 0)
-                return log_debug_errno(r, "Failed to execute operation: %s", bus_error_message(&error, r));
-
-        return 1;
-}
-
 static int execute_s2h(const SleepConfig *sleep_config) {
-        _unused_ _cleanup_(freeze_thaw_user_slice) const char *auto_method_thaw = "ThawUnit";
         int r;
 
         assert(sleep_config);
 
-        r = freeze_thaw_user_slice(&(const char*) { "FreezeUnit" });
-        if (r < 0)
-                log_warning_errno(r, "Failed to freeze unit user.slice, ignoring: %m");
-
         /* Only check if we have automated battery alarms if HibernateDelaySec= is not set, as in that case
          * we'll busy poll for the configured interval instead */
         if (!timestamp_is_set(sleep_config->hibernate_delay_usec)) {
@@ -599,6 +574,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_(sleep_config_freep) SleepConfig *sleep_config = NULL;
         int r;
 
@@ -617,6 +593,22 @@ static int run(int argc, char *argv[]) {
                                        "Sleep operation \"%s\" is disabled by configuration, refusing.",
                                        sleep_operation_to_string(arg_operation));
 
+        /* Freeze the user sessions */
+        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 sysupend-then-hibernate "
+                           "operations or setups with encrypted home directories.");
+
         switch (arg_operation) {
 
         case SLEEP_SUSPEND_THEN_HIBERNATE: