Hello, We restart logind in our systemd package on package upgrades, as that should generally be safe and our policy is to do that whenever possible to avoid reboots and apply security and other fixes immediately.
We got several reports (https://launchpad.net/bugs/1415104) about users losing their device ACLs and polkit privileges when they install package updates involving systemd. I found out that this happens if you have older sessions around in state "closing", i. e. there's still some leftover process in them [1]. After restarting logind, that old "closing" session suddenly becomes Active=yes, and the "real" active session becomes Active=no. Patch attached, I tested it fairly thoroughly. Thanks for considering! Martin [1] In many cases that's of course a bug in its own right. E. g. pulseaudio ought to die with the session instead of keeping it open, but there's also legitimate use cases like screen. -- Martin Pitt | http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From 6706e1bfc69eb2cc5e3886839e71ab9d35d57627 Mon Sep 17 00:00:00 2001 From: Martin Pitt <[email protected]> Date: Wed, 28 Jan 2015 18:14:01 +0100 Subject: [PATCH] logind: handle closing sessions over daemon restarts It may happen that you have several sessions with the same VT: - Open a session c1 which leaves some processes around, and log out. The session will stay in State=closing and become Active=no. - Log back in on the same VT, get a new session "c2" which is State=active and Active=yes. When restarting logind after that, the first session that matches the current VT becomes Active=yes, which will be c1; c2 thus is Active=no and does not get the usual polkit/device ACL privileges. Restore the "closing" state in session_load(), to avoid treating all restored sessions as State=active. In seat_active_vt_changed(), prefer active sessions over closing ones if more than one session matches the current VT. Finally, fix the confusing comment in session_load() and explain it a bit better. https://launchpad.net/bugs/1415104 --- src/login/logind-seat.c | 14 +++++++++++++- src/login/logind-session.c | 11 +++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 197138c..126c5b8 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -340,12 +340,24 @@ int seat_active_vt_changed(Seat *s, unsigned int vtnr) { log_debug("VT changed to %u", vtnr); + /* we might have earlier closing sessions on the same VT, so try to + * find a running one first */ LIST_FOREACH(sessions_by_seat, i, s->sessions) - if (i->vtnr == vtnr) { + if (i->vtnr == vtnr && !i->stopping) { new_active = i; break; } + if (!new_active) { + /* no running one? then we can't decide which one is the + * active one, let the first one win */ + LIST_FOREACH(sessions_by_seat, i, s->sessions) + if (i->vtnr == vtnr) { + new_active = i; + break; + } + } + r = seat_set_active(s, new_active); manager_spawn_autovt(s->manager, vtnr); diff --git a/src/login/logind-session.c b/src/login/logind-session.c index a51f9f3..a02a537 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -301,6 +301,7 @@ int session_load(Session *s) { _cleanup_free_ char *remote = NULL, *seat = NULL, *vtnr = NULL, + *state = NULL, *pos = NULL, *leader = NULL, *type = NULL, @@ -327,6 +328,7 @@ int session_load(Session *s) { "SERVICE", &s->service, "DESKTOP", &s->desktop, "VTNR", &vtnr, + "STATE", &state, "POS", &pos, "LEADER", &leader, "TYPE", &type, @@ -415,13 +417,18 @@ int session_load(Session *s) { s->class = c; } + if (state && streq(state, "closing")) + s->stopping = true; + if (s->fifo_path) { int fd; /* If we open an unopened pipe for reading we will not get an EOF. to trigger an EOF we hence open it for - reading, but close it right-away which then will - trigger the EOF. */ + writing, but close it right away which then will + trigger the EOF. This will happen immediately if no + other process has the FIFO open for writing, i. e. + when the session died before logind (re)started. */ fd = session_create_fifo(s); safe_close(fd); -- 2.1.4
signature.asc
Description: Digital signature
_______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
