logind: restore logic to kill user processes when session ends

9 files changed, 157 insertions, 130 deletions

diff --git a/man/logind.conf.xml b/man/logind.conf.xml
index 8ab6d729a9..54cc379048 100644
--- a/man/logind.conf.xml
+++ b/man/logind.conf.xml
@@ -138,7 +138,34 @@
                                 processes of a user should be killed
                                 when she or he completely logs out (i.e. after
                                 her/his last session ended). Defaults to
-                                <literal>no</literal>.</para></listitem>
+                                <literal>no</literal>.</para>
+
+                                <para>Note that setting
+                                <varname>KillUserProcesses=1</varname>
+                                will break tools like
+                                <citerefentry><refentrytitle>screen</refentrytitle><manvolnum>1</manvolnum></citerefentry>.</para></listitem>
+                        </varlistentry>
+
+                        <varlistentry>
+                                <term><varname>KillOnlyUsers=</varname></term>
+                                <term><varname>KillExcludeUsers=</varname></term>
+
+                                <listitem><para>These settings take
+                                space-separated lists of usernames
+                                that influence the effect of
+                                <varname>KillUserProcesses=</varname>. If
+                                not empty, only processes of users
+                                listed in
+                                <varname>KillOnlyUsers=</varname> will
+                                be killed when they log out
+                                entirely. Processes of users listed in
+                                <varname>KillExcludeUsers=</varname>
+                                are excluded from being
+                                killed. <varname>KillExcludeUsers=</varname>
+                                defaults to <literal>root</literal>
+                                and takes precedence over
+                                <varname>KillOnlyUsers=</varname>,
+                                which defaults to the empty list.</para></listitem>
                         </varlistentry>
 
                         <varlistentry>
@@ -180,64 +207,6 @@
                         </varlistentry>
 
                         <varlistentry>
-                                <term><varname>KillOnlyUsers=</varname></term>
-                                <term><varname>KillExcludeUsers=</varname></term>
-
-                                <listitem><para>These settings take
-                                space-separated lists of usernames
-                                that influence the effect of
-                                <varname>KillUserProcesses=</varname>. If
-                                not empty, only processes of users
-                                listed in
-                                <varname>KillOnlyUsers</varname> will
-                                be killed when they log out
-                                entirely. Processes of users listed in
-                                <varname>KillExcludeUsers=</varname>
-                                are excluded from being
-                                killed. <varname>KillExcludeUsers=</varname>
-                                defaults to <literal>root</literal>
-                                and takes precedence over
-                                <varname>KillOnlyUsers=</varname>,
-                                which defaults to the empty list.</para></listitem>
-                        </varlistentry>
-
-                        <varlistentry>
-                                <term><varname>Controllers=</varname></term>
-                                <term><varname>ResetControllers=</varname></term>
-
-                                <listitem><para>These settings control
-                                the default control group hierarchies
-                                users logging in are added to, in
-                                addition to the
-                                <literal>name=systemd</literal> named
-                                hierarchy. These settings take
-                                space-separated lists of controller
-                                names. Pass the empty string to ensure
-                                that logind does not touch any
-                                hierarchies but systemd's own. When
-                                logging in, user sessions will get
-                                private control groups in all
-                                hierarchies listed in
-                                <varname>Controllers=</varname> and be
-                                reset to the root control group in all
-                                hierarchies listed in
-                                <varname>ResetControllers=</varname>.
-                                <varname>Controllers=</varname>
-                                defaults to the empty list.
-                                <varname>ResetControllers=</varname>
-                                defaults to
-                                <literal>cpu</literal>. Note that for
-                                all controllers that are not listed in
-                                either <varname>Controllers=</varname>
-                                or
-                                <varname>ResetControllers=</varname>,
-                                newly created sessions will be part of
-                                the control groups of the system
-                                service that created the
-                                session.</para></listitem>
-                        </varlistentry>
-
-                        <varlistentry>
                                 <term><varname>InhibitDelayMaxSec=</varname></term>
 
                                 <listitem><para>Specifies the maximum
@@ -323,20 +292,6 @@
                         </varlistentry>
 
                 </variablelist>
-
-                <para>Note that setting
-                <varname>KillUserProcesses=1</varname> will break tools
-                like
-                <citerefentry><refentrytitle>screen</refentrytitle><manvolnum>1</manvolnum></citerefentry>.</para>
-
-                <para>Note that <varname>KillUserProcesses=1</varname>
-                is a weaker version of
-                <varname>kill-session-processes=1</varname>, which may
-                be configured per-service for
-                <citerefentry><refentrytitle>pam_systemd</refentrytitle><manvolnum>8</manvolnum></citerefentry>. The
-                latter kills processes of a session as soon as it
-                ends, the former kills processes as soon as the last
-                session of the user ends.</para>
         </refsect1>
 
         <refsect1>
diff --git a/src/core/dbus-kill.c b/src/core/dbus-kill.c
index 80e15e3fca..811adb1b5a 100644
--- a/src/core/dbus-kill.c
+++ b/src/core/dbus-kill.c
@@ -48,7 +48,28 @@ int bus_kill_context_set_transient_property(
         assert(name);
         assert(i);
 
-        if (streq(name, "SendSIGHUP")) {
+        if (streq(name, "KillMode")) {
+                const char *m;
+                KillMode k;
+
+                if (dbus_message_iter_get_arg_type(i) != DBUS_TYPE_STRING)
+                        return -EINVAL;
+
+                dbus_message_iter_get_basic(i, &m);
+
+                k = kill_mode_from_string(m);
+                if (k < 0)
+                        return -EINVAL;
+
+                if (mode != UNIT_CHECK) {
+                        c->kill_mode = k;
+
+                        unit_write_drop_in_private_format(u, mode, name, "KillMode=%s\n", kill_mode_to_string(k));
+                }
+
+                return 1;
+
+        } else if (streq(name, "SendSIGHUP")) {
 
                 if (dbus_message_iter_get_arg_type(i) != DBUS_TYPE_BOOLEAN)
                         return -EINVAL;
@@ -59,7 +80,7 @@ int bus_kill_context_set_transient_property(
                         dbus_message_iter_get_basic(i, &b);
                         c->send_sighup = b;
 
-                        unit_write_drop_in_format(u, mode, name, "[Scope]\nSendSIGHUP=%s\n", yes_no(b));
+                        unit_write_drop_in_private_format(u, mode, name, "SendSIGHUP=%s\n", yes_no(b));
                 }
 
                 return 1;
@@ -75,7 +96,7 @@ int bus_kill_context_set_transient_property(
                         dbus_message_iter_get_basic(i, &b);
                         c->send_sigkill = b;
 
-                        unit_write_drop_in_format(u, mode, name, "[Scope]\nSendSIGKILL4=%s\n", yes_no(b));
+                        unit_write_drop_in_private_format(u, mode, name, "SendSIGKILL=%s\n", yes_no(b));
                 }
 
                 return 1;
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index ed5d8d888c..345df9f1cc 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2523,6 +2523,7 @@ int manager_start_scope(
                 const char *slice,
                 const char *description,
                 const char *after,
+                const char *kill_mode,
                 DBusError *error,
                 char **job) {
 
@@ -2594,6 +2595,18 @@ int manager_start_scope(
                         return log_oom();
         }
 
+        if (!isempty(kill_mode)) {
+                const char *kill_mode_property = "KillMode";
+
+                if (!dbus_message_iter_open_container(&sub, DBUS_TYPE_STRUCT, NULL, &sub2) ||
+                    !dbus_message_iter_append_basic(&sub2, DBUS_TYPE_STRING, &kill_mode_property) ||
+                    !dbus_message_iter_open_container(&sub2, DBUS_TYPE_VARIANT, "s", &sub3) ||
+                    !dbus_message_iter_append_basic(&sub3, DBUS_TYPE_STRING, &kill_mode) ||
+                    !dbus_message_iter_close_container(&sub2, &sub3) ||
+                    !dbus_message_iter_close_container(&sub, &sub2))
+                        return log_oom();
+        }
+
         /* cgroup empty notification is not available in containers
          * currently. To make this less problematic, let's shorten the
          * stop timeout for sessions, so that we don't wait
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index db22150825..1fea4745b6 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -32,7 +32,6 @@
 #include "util.h"
 #include "mkdir.h"
 #include "path-util.h"
-#include "cgroup-util.h"
 #include "fileio.h"
 #include "dbus-common.h"
 #include "logind-session.h"
@@ -466,15 +465,20 @@ static int session_start_scope(Session *s) {
 
         if (!s->scope) {
                 _cleanup_free_ char *description = NULL;
+                const char *kill_mode;
                 char *scope, *job;
 
+                description = strjoin("Session ", s->id, " of user ", s->user->name, NULL);
+                if (!description)
+                        return log_oom();
+
                 scope = strjoin("session-", s->id, ".scope", NULL);
                 if (!scope)
                         return log_oom();
 
-                description = strjoin("Session ", s->id, " of user ", s->user->name, NULL);
+                kill_mode = manager_shall_kill(s->manager, s->user->name) ? "control-group" : "none";
 
-                r = manager_start_scope(s->manager, scope, s->leader, s->user->slice, description, "systemd-user-sessions.service", &error, &job);
+                r = manager_start_scope(s->manager, scope, s->leader, s->user->slice, description, "systemd-user-sessions.service", kill_mode, &error, &job);
                 if (r < 0) {
                         log_error("Failed to start session scope: %s %s", bus_error(&error, r), error.name);
                         dbus_error_free(&error);
@@ -554,21 +558,6 @@ int session_start(Session *s) {
         return 0;
 }
 
-/* static bool session_shall_kill(Session *s) { */
-/*         assert(s); */
-
-/*         if (!s->kill_processes) */
-/*                 return false; */
-
-/*         if (strv_contains(s->manager->kill_exclude_users, s->user->name)) */
-/*                 return false; */
-
-/*         if (strv_isempty(s->manager->kill_only_users)) */
-/*                 return true; */
-
-/*         return strv_contains(s->manager->kill_only_users, s->user->name); */
-/* } */
-
 static int session_stop_scope(Session *s) {
         DBusError error;
         char *job;
@@ -617,7 +606,23 @@ static int session_unlink_x11_socket(Session *s) {
 }
 
 int session_stop(Session *s) {
-        int r = 0, k;
+        int r;
+
+        assert(s);
+
+        if (!s->user)
+                return -ESTALE;
+
+        /* Kill cgroup */
+        r = session_stop_scope(s);
+
+        session_save(s);
+
+        return r;
+}
+
+int session_finalize(Session *s) {
+        int r = 0;
 
         assert(s);
 
@@ -633,11 +638,6 @@ int session_stop(Session *s) {
                            "MESSAGE=Removed session %s.", s->id,
                            NULL);
 
-        /* Kill cgroup */
-        k = session_stop_scope(s);
-        if (k < 0)
-                r = k;
-
         /* Remove X11 symlink */
         session_unlink_x11_socket(s);
 
@@ -645,10 +645,10 @@ int session_stop(Session *s) {
         session_add_to_gc_queue(s);
         user_add_to_gc_queue(s->user);
 
-        if (s->started)
+        if (s->started) {
                 session_send_signal(s, false);
-
-        s->started = false;
+                s->started = false;
+        }
 
         if (s->seat) {
                 if (s->seat->active == s)
@@ -871,7 +871,6 @@ int session_check_gc(Session *s, bool drop_not_started) {
                 return 0;
 
         if (s->fifo_fd >= 0) {
-
                 r = pipe_eof(s->fifo_fd);
                 if (r < 0)
                         return r;
@@ -902,8 +901,11 @@ void session_add_to_gc_queue(Session *s) {
 SessionState session_get_state(Session *s) {
         assert(s);
 
+        if (s->closing)
+                return SESSION_CLOSING;
+
         if (s->scope_job)
-                return s->started ? SESSION_OPENING : SESSION_CLOSING;
+                return SESSION_OPENING;
 
         if (s->fifo_fd < 0)
                 return SESSION_CLOSING;
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index e2a46d5907..edaae8d20a 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -101,6 +101,7 @@ struct Session {
 
         bool in_gc_queue:1;
         bool started:1;
+        bool closing:1;
 
         DBusMessage *create_message;
 
@@ -123,6 +124,7 @@ int session_create_fifo(Session *s);
 void session_remove_fifo(Session *s);
 int session_start(Session *s);
 int session_stop(Session *s);
+int session_finalize(Session *s);
 int session_save(Session *s);
 int session_load(Session *s);
 int session_kill(Session *s, KillWho who, int signo);
diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index 0a985a53ec..adbe638d46 100644
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -490,21 +490,6 @@ static int user_stop_service(User *u) {
         return r;
 }
 
-/* static int user_shall_kill(User *u) { */
-/*         assert(u); */
-
-/*         if (!u->manager->kill_user_processes) */
-/*                 return false; */
-
-/*         if (strv_contains(u->manager->kill_exclude_users, u->name)) */
-/*                 return false; */
-
-/*         if (strv_isempty(u->manager->kill_only_users)) */
-/*                 return true; */
-
-/*         return strv_contains(u->manager->kill_only_users, u->name); */
-/* } */
-
 static int user_remove_runtime_path(User *u) {
         int r;
 
@@ -528,9 +513,6 @@ int user_stop(User *u) {
         int r = 0, k;
         assert(u);
 
-        if (u->started)
-                log_debug("User %s logged out.", u->name);
-
         LIST_FOREACH(sessions_by_user, s, u->sessions) {
                 k = session_stop(s);
                 if (k < 0)
@@ -547,6 +529,26 @@ int user_stop(User *u) {
         if (k < 0)
                 r = k;
 
+        user_save(u);
+
+        return r;
+}
+
+int user_finalize(User *u) {
+        Session *s;
+        int r = 0, k;
+
+        assert(u);
+
+        if (u->started)
+                log_debug("User %s logged out.", u->name);
+
+        LIST_FOREACH(sessions_by_user, s, u->sessions) {
+                k = session_finalize(s);
+                if (k < 0)
+                        r = k;
+        }
+
         /* Kill XDG_RUNTIME_DIR */
         k = user_remove_runtime_path(u);
         if (k < 0)
@@ -555,10 +557,10 @@ int user_stop(User *u) {
         unlink(u->state_file);
         user_add_to_gc_queue(u);
 
-        if (u->started)
+        if (u->started) {
                 user_send_signal(u, false);
-
-        u->started = false;
+                u->started = false;
+        }
 
         return r;
 }
@@ -624,6 +626,15 @@ int user_check_gc(User *u, bool drop_not_started) {
         if (user_check_linger_file(u) > 0)
                 return 1;
 
+        if (u->slice_job || u->service_job)
+                return 1;
+
+        if (u->slice && manager_unit_is_active(u->manager, u->slice) != 0)
+                return 1;
+
+        if (u->service && manager_unit_is_active(u->manager, u->service) != 0)
+                return 1;
+
         return 0;
 }
 
@@ -643,8 +654,11 @@ UserState user_get_state(User *u) {
 
         assert(u);
 
+        if (u->closing)
+                return USER_CLOSING;
+
         if (u->slice_job || u->service_job)
-                return u->started ? USER_OPENING : USER_CLOSING;
+                return USER_OPENING;
 
         LIST_FOREACH(sessions_by_user, i, u->sessions) {
                 if (session_is_active(i))
diff --git a/src/login/logind-user.h b/src/login/logind-user.h
index 889f828c42..b9171d345d 100644
--- a/src/login/logind-user.h
+++ b/src/login/logind-user.h
@@ -61,8 +61,7 @@ struct User {
 
         bool in_gc_queue:1;
         bool started:1;
-        bool slice_created:1;
-        bool service_created:1;
+        bool closing:1;
 
         LIST_HEAD(Session, sessions);
         LIST_FIELDS(User, gc_queue);
@@ -74,6 +73,7 @@ int user_check_gc(User *u, bool drop_not_started);
 void user_add_to_gc_queue(User *u);
 int user_start(User *u);
 int user_stop(User *u);
+int user_finalize(User *u);
 UserState user_get_state(User *u);
 int user_get_idle_hint(User *u, dual_timestamp *t);
 int user_save(User *u);
diff --git a/src/login/logind.c b/src/login/logind.c
index a79ba333d2..0002d262c1 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -1244,6 +1244,7 @@ void manager_gc(Manager *m, bool drop_not_started) {
 
                 if (session_check_gc(session, drop_not_started) == 0) {
                         session_stop(session);
+                        session_finalize(session);
                         session_free(session);
                 }
         }
@@ -1254,6 +1255,7 @@ void manager_gc(Manager *m, bool drop_not_started) {
 
                 if (user_check_gc(user, drop_not_started) == 0) {
                         user_stop(user);
+                        user_finalize(user);
                         user_free(user);
                 }
         }
@@ -1298,6 +1300,22 @@ int manager_get_idle_hint(Manager *m, dual_timestamp *t) {
         return idle_hint;
 }
 
+bool manager_shall_kill(Manager *m, const char *user) {
+        assert(m);
+        assert(user);
+
+        if (!m->kill_user_processes)
+                return false;
+
+        if (strv_contains(m->kill_exclude_users, user))
+                return false;
+
+        if (strv_isempty(m->kill_only_users))
+                return true;
+
+        return strv_contains(m->kill_only_users, user);
+}
+
 int manager_dispatch_idle_action(Manager *m) {
         struct dual_timestamp since;
         struct itimerspec its = {};
diff --git a/src/login/logind.h b/src/login/logind.h
index 9c41cdf70c..e9838a8afd 100644
--- a/src/login/logind.h
+++ b/src/login/logind.h
@@ -163,6 +163,8 @@ int manager_spawn_autovt(Manager *m, int vtnr);
 
 void manager_gc(Manager *m, bool drop_not_started);
 
+bool manager_shall_kill(Manager *m, const char *user);
+
 int manager_get_idle_hint(Manager *m, dual_timestamp *t);
 
 int manager_get_user_by_pid(Manager *m, pid_t pid, User **user);
@@ -178,7 +180,7 @@ int manager_send_changed(Manager *manager, const char *properties);
 
 int manager_dispatch_delayed(Manager *manager);
 
-int manager_start_scope(Manager *manager, const char *scope, pid_t pid, const char *slice, const char *description, const char *after, DBusError *error, char **job);
+int manager_start_scope(Manager *manager, const char *scope, pid_t pid, const char *slice, const char *description, const char *after, const char *kill_mode, DBusError *error, char **job);
 int manager_start_unit(Manager *manager, const char *unit, DBusError *error, char **job);
 int manager_stop_unit(Manager *manager, const char *unit, DBusError *error, char **job);
 int manager_kill_unit(Manager *manager, const char *unit, KillWho who, int signo, DBusError *error);