[systemd-devel] [PATCH 3/3] logind: session: Fix not allowing more than one controller

Olivier Brunel jjk at jjacky.com
Fri Aug 8 11:45:44 PDT 2014


While a session can only ever have one controller, there can be more than
one session with a controller at a time. However, because of the handling
of SIGUSR1 for handling VT switch, trying to set a controller on a session
while another session had a controller would fail.

E.g. trying to start a rootless X while there's already one running on
another session would fail, as its request to TakeControl() would fail,
because we couldn't set a signal handler for SIGUSR1 (since we've already
one set up for the other session).

When a session goes inactive, we remove the handler (since it cannot be
deactivated anymore), thus allowing to set one up for another session if needed.
Of course, when the session is activated again, we reset the handler.

(Should be noted that our handler might also never be called, if e.g. the
controller did a VT_SETMODE as well, thus handling/receiving the SIGUSR1)

Alas, if there can be two sessions active at the same time (in different
seats) then the second TakeControl() request will fail, for the same
reason. This could probably be fixed by making sd-event support multiple signal
handlers per signal.

https://bugs.freedesktop.org/show_bug.cgi?id=81932
---
Note that when I say trying to start another rootless X would have its
TakeControl() to fail, that's only true with the previous patch. Today it
actually succeeds, but then X fails with a permission denied error when it tries
to access /dev/ttyX, as session_restore_vt() was called and, amongst other
things, the owner was reset to root.


 src/login/logind-seat.c    |  9 +++++++++
 src/login/logind-session.c | 39 ++++++++++++++++++++++++++++++---------
 src/login/logind-session.h |  2 ++
 3 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 9992195..1fb0d68 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -239,6 +239,8 @@ int seat_set_active(Seat *s, Session *session) {
         s->active = session;
 
         if (old_active) {
+                if (old_active->controller)
+                        session_vt_deactivate(old_active);
                 session_device_pause_all(old_active);
                 session_send_changed(old_active, "Active", NULL);
         }
@@ -246,6 +248,13 @@ int seat_set_active(Seat *s, Session *session) {
         seat_apply_acls(s, old_active);
 
         if (session && session->started) {
+                if (session->controller) {
+                        int r;
+
+                        r = session_vt_activate(session);
+                        if (r < 0)
+                                log_error("failed to activate VT %u for session %s (%d/%d)", session->vtnr, session->id, r, errno);
+                }
                 session_send_changed(session, "Active", NULL);
                 session_device_resume_all(session);
         }
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 3f4e177..7428e6c 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -1003,10 +1003,35 @@ static int session_vt_fn(sd_event_source *source, const struct signalfd_siginfo
         return 0;
 }
 
+int session_vt_activate(Session *s) {
+        int r;
+        sigset_t mask;
+
+        sigemptyset(&mask);
+        sigaddset(&mask, SIGUSR1);
+        sigprocmask(SIG_BLOCK, &mask, NULL);
+
+        r = sd_event_add_signal(s->manager->event, &s->vt_source, SIGUSR1, session_vt_fn, s);
+        if (r < 0)
+            return r;
+
+        return 0;
+}
+
+void session_vt_deactivate(Session *s) {
+        assert(s);
+
+        /* This is called once the session is inactive, and therefore the
+         * handler isn't needed anymore, and will be reset when the session is
+         * activated again.
+         * Meanwhile, it allows to set up another handler for another session if
+         * needed. */
+        s->vt_source = sd_event_source_unref(s->vt_source);
+}
+
 int session_prepare_vt(Session *s) {
         int vt, r;
         struct vt_mode mode = { 0 };
-        sigset_t mask;
 
         vt = session_open_vt(s);
         if (vt < 0)
@@ -1024,17 +1049,13 @@ int session_prepare_vt(Session *s) {
         if (r < 0)
                 goto error;
 
-        sigemptyset(&mask);
-        sigaddset(&mask, SIGUSR1);
-        sigprocmask(SIG_BLOCK, &mask, NULL);
-
-        r = sd_event_add_signal(s->manager->event, &s->vt_source, SIGUSR1, session_vt_fn, s);
-        if (r < 0)
-                goto error;
-
         /* Oh, thanks to the VT layer, VT_AUTO does not work with KD_GRAPHICS.
          * So we need a dummy handler here which just acknowledges *all* VT
          * switch requests. */
+        r = session_vt_activate(s);
+        if (r < 0)
+                goto error;
+
         mode.mode = VT_PROCESS;
         mode.relsig = SIGUSR1;
         mode.acqsig = SIGUSR1;
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index 2ab3182..5ad2f26 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -172,6 +172,8 @@ SessionClass session_class_from_string(const char *s) _pure_;
 const char *kill_who_to_string(KillWho k) _const_;
 KillWho kill_who_from_string(const char *s) _pure_;
 
+int session_vt_activate(Session *s);
+void session_vt_deactivate(Session *s);
 int session_prepare_vt(Session *s);
 void session_restore_vt(Session *s);
 
-- 
2.0.4



More information about the systemd-devel mailing list