From 8c29a4570993105fecc12288596d2ee77c7f82b8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 3 Aug 2018 18:53:09 +0200 Subject: [PATCH 01/37] logind: rework Seat/Session/User object allocation and freeing a bit Let's update things a bit to follow current practices: - User structure initialization rather than zero-initialized allocation - Always propagate proper errors from allocation functions - Use _cleanup_ for freeing objects when allocation fails half-way - Make destructors return NULL --- src/login/logind-core.c | 14 ++++++---- src/login/logind-seat.c | 38 +++++++++++++++---------- src/login/logind-seat.h | 6 ++-- src/login/logind-session.c | 57 +++++++++++++++++++++----------------- src/login/logind-session.h | 7 +++-- src/login/logind-user.c | 21 +++++++------- 6 files changed, 83 insertions(+), 60 deletions(-) diff --git a/src/login/logind-core.c b/src/login/logind-core.c index 87506088a1..3fe2838fc2 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -90,15 +90,16 @@ int manager_add_device(Manager *m, const char *sysfs, bool master, Device **_dev int manager_add_seat(Manager *m, const char *id, Seat **_seat) { Seat *s; + int r; assert(m); assert(id); s = hashmap_get(m->seats, id); if (!s) { - s = seat_new(m, id); - if (!s) - return -ENOMEM; + r = seat_new(&s, m, id); + if (r < 0) + return r; } if (_seat) @@ -109,15 +110,16 @@ int manager_add_seat(Manager *m, const char *id, Seat **_seat) { int manager_add_session(Manager *m, const char *id, Session **_session) { Session *s; + int r; assert(m); assert(id); s = hashmap_get(m->sessions, id); if (!s) { - s = session_new(m, id); - if (!s) - return -ENOMEM; + r = session_new(&s, m, id); + if (r < 0) + return r; } if (_session) diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index f24fe96841..398c74d712 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -21,33 +21,42 @@ #include "terminal-util.h" #include "util.h" -Seat *seat_new(Manager *m, const char *id) { - Seat *s; +int seat_new(Seat** ret, Manager *m, const char *id) { + _cleanup_(seat_freep) Seat *s = NULL; + int r; + assert(ret); assert(m); assert(id); - s = new0(Seat, 1); + if (!seat_name_is_valid(id)) + return -EINVAL; + + s = new(Seat, 1); if (!s) - return NULL; + return -ENOMEM; + + *s = (Seat) { + .manager = m, + }; s->state_file = strappend("/run/systemd/seats/", id); if (!s->state_file) - return mfree(s); + return -ENOMEM; s->id = basename(s->state_file); - s->manager = m; - if (hashmap_put(m->seats, s->id, s) < 0) { - free(s->state_file); - return mfree(s); - } + r = hashmap_put(m->seats, s->id, s); + if (r < 0) + return r; - return s; + *ret = TAKE_PTR(s); + return 0; } -void seat_free(Seat *s) { - assert(s); +Seat* seat_free(Seat *s) { + if (!s) + return NULL; if (s->in_gc_queue) LIST_REMOVE(gc_queue, s->manager->seat_gc_queue, s); @@ -64,7 +73,8 @@ void seat_free(Seat *s) { free(s->positions); free(s->state_file); - free(s); + + return mfree(s); } int seat_save(Seat *s) { diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h index 70878bbe52..51cd468e26 100644 --- a/src/login/logind-seat.h +++ b/src/login/logind-seat.h @@ -27,8 +27,10 @@ struct Seat { LIST_FIELDS(Seat, gc_queue); }; -Seat *seat_new(Manager *m, const char *id); -void seat_free(Seat *s); +int seat_new(Seat **ret, Manager *m, const char *id); +Seat* seat_free(Seat *s); + +DEFINE_TRIVIAL_CLEANUP_FUNC(Seat *, seat_free); int seat_save(Seat *s); int seat_load(Seat *s); diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 226cc49b01..f59e62837e 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -24,57 +24,61 @@ #include "mkdir.h" #include "parse-util.h" #include "path-util.h" +#include "process-util.h" #include "string-table.h" #include "terminal-util.h" #include "user-util.h" #include "util.h" -#include "process-util.h" #define RELEASE_USEC (20*USEC_PER_SEC) static void session_remove_fifo(Session *s); -Session* session_new(Manager *m, const char *id) { - Session *s; +int session_new(Session **ret, Manager *m, const char *id) { + _cleanup_(session_freep) Session *s = NULL; + int r; + assert(ret); assert(m); assert(id); - assert(session_id_valid(id)); - s = new0(Session, 1); + if (!session_id_valid(id)) + return -EINVAL; + + s = new(Session, 1); if (!s) - return NULL; + return -ENOMEM; + + *s = (Session) { + .manager = m, + .fifo_fd = -1, + .vtfd = -1, + .audit_id = AUDIT_SESSION_INVALID, + }; s->state_file = strappend("/run/systemd/sessions/", id); if (!s->state_file) - return mfree(s); - - s->devices = hashmap_new(&devt_hash_ops); - if (!s->devices) { - free(s->state_file); - return mfree(s); - } + return -ENOMEM; s->id = basename(s->state_file); - if (hashmap_put(m->sessions, s->id, s) < 0) { - hashmap_free(s->devices); - free(s->state_file); - return mfree(s); - } + s->devices = hashmap_new(&devt_hash_ops); + if (!s->devices) + return -ENOMEM; - s->manager = m; - s->fifo_fd = -1; - s->vtfd = -1; - s->audit_id = AUDIT_SESSION_INVALID; + r = hashmap_put(m->sessions, s->id, s); + if (r < 0) + return r; - return s; + *ret = TAKE_PTR(s); + return 0; } -void session_free(Session *s) { +Session* session_free(Session *s) { SessionDevice *sd; - assert(s); + if (!s) + return NULL; if (s->in_gc_queue) LIST_REMOVE(gc_queue, s->manager->session_gc_queue, s); @@ -126,7 +130,8 @@ void session_free(Session *s) { hashmap_remove(s->manager->sessions, s->id); free(s->state_file); - free(s); + + return mfree(s); } void session_set_user(Session *s, User *u) { diff --git a/src/login/logind-session.h b/src/login/logind-session.h index 29ca399daf..572f2545c1 100644 --- a/src/login/logind-session.h +++ b/src/login/logind-session.h @@ -109,8 +109,11 @@ struct Session { LIST_FIELDS(Session, gc_queue); }; -Session *session_new(Manager *m, const char *id); -void session_free(Session *s); +int session_new(Session **ret, Manager *m, const char *id); +Session* session_free(Session *s); + +DEFINE_TRIVIAL_CLEANUP_FUNC(Session *, session_free); + void session_set_user(Session *s, User *u); bool session_may_gc(Session *s, bool drop_not_started); void session_add_to_gc_queue(Session *s); diff --git a/src/login/logind-user.c b/src/login/logind-user.c index f2664c323e..ad6349944c 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -30,23 +30,24 @@ #include "user-util.h" #include "util.h" -int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name) { +int user_new(User **ret, Manager *m, uid_t uid, gid_t gid, const char *name) { _cleanup_(user_freep) User *u = NULL; char lu[DECIMAL_STR_MAX(uid_t) + 1]; int r; - assert(out); + assert(ret); assert(m); assert(name); - u = new0(User, 1); + u = new(User, 1); if (!u) return -ENOMEM; - u->manager = m; - u->uid = uid; - u->gid = gid; - xsprintf(lu, UID_FMT, uid); + *u = (User) { + .manager = m, + .uid = uid, + .gid = gid, + }; u->name = strdup(name); if (!u->name) @@ -58,6 +59,7 @@ int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name) { if (asprintf(&u->runtime_path, "/run/user/"UID_FMT, uid) < 0) return -ENOMEM; + xsprintf(lu, UID_FMT, uid); r = slice_build_subslice(SPECIAL_USER_SLICE, lu, &u->slice); if (r < 0) return r; @@ -78,8 +80,7 @@ int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name) { if (r < 0) return r; - *out = TAKE_PTR(u); - + *ret = TAKE_PTR(u); return 0; } @@ -272,7 +273,7 @@ int user_save(User *u) { if (!u->started) return 0; - return user_save_internal (u); + return user_save_internal(u); } int user_load(User *u) { From 1c8280fd47b6561d35b15b3b6d49bdeacf891bfd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 3 Aug 2018 19:04:35 +0200 Subject: [PATCH 02/37] logind: fix serialization/deserialization of user's "display session" Previously this was serialized as part of the user object. This didn't work however, as we load users first, and sessions seconds and hence referencing a session from the user load logic cannot work. Fix this by storing an IS_DISPLAY property along with each session, and make the session with this set display session when it is loaded. --- src/login/logind-session.c | 18 +++++++++++++++++- src/login/logind-user.c | 18 ++++-------------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index f59e62837e..6c447f5534 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -184,11 +184,13 @@ int session_save(Session *s) { "UID="UID_FMT"\n" "USER=%s\n" "ACTIVE=%i\n" + "IS_DISPLAY=%i\n" "STATE=%s\n" "REMOTE=%i\n", s->user->uid, s->user->name, session_is_active(s), + s->user->display == s, session_state_to_string(session_get_state(s)), s->remote); @@ -359,7 +361,8 @@ int session_load(Session *s) { *monotonic = NULL, *controller = NULL, *active = NULL, - *devices = NULL; + *devices = NULL, + *is_display = NULL; int k, r; @@ -389,6 +392,7 @@ int session_load(Session *s) { "CONTROLLER", &controller, "ACTIVE", &active, "DEVICES", &devices, + "IS_DISPLAY", &is_display, NULL); if (r < 0) @@ -496,6 +500,18 @@ int session_load(Session *s) { s->was_active = k; } + if (is_display) { + /* Note that when enumerating users are loaded before sessions, hence the display session to use is + * something we have to store along with the session and not the user, as in that case we couldn't + * apply it at the time we load the user. */ + + k = parse_boolean(is_display); + if (k < 0) + log_warning_errno(k, "Failed to parse IS_DISPLAY session property: %m"); + else if (k > 0) + s->user->display = s; + } + if (controller) { if (bus_name_has_owner(s->manager->bus, controller, NULL) > 0) { session_set_controller(s, controller, false, false); diff --git a/src/login/logind-user.c b/src/login/logind-user.c index ad6349944c..302d3b08e6 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -277,8 +277,7 @@ int user_save(User *u) { } int user_load(User *u) { - _cleanup_free_ char *display = NULL, *realtime = NULL, *monotonic = NULL; - Session *s = NULL; + _cleanup_free_ char *realtime = NULL, *monotonic = NULL; int r; assert(u); @@ -286,22 +285,13 @@ int user_load(User *u) { r = parse_env_file(NULL, u->state_file, NEWLINE, "SERVICE_JOB", &u->service_job, "SLICE_JOB", &u->slice_job, - "DISPLAY", &display, "REALTIME", &realtime, "MONOTONIC", &monotonic, NULL); - if (r < 0) { - if (r == -ENOENT) - return 0; - + if (r == -ENOENT) + return 0; + if (r < 0) return log_error_errno(r, "Failed to read %s: %m", u->state_file); - } - - if (display) - s = hashmap_get(u->manager->sessions, display); - - if (s && s->display && display_is_local(s->display)) - u->display = s; if (realtime) timestamp_deserialize(realtime, &u->timestamp.realtime); From 44176400138e18d9087e0864ca97041416a90d47 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 3 Aug 2018 20:18:55 +0200 Subject: [PATCH 03/37] logind: turn of stdio locking when writing session files too This just copies what we already do for user and seat files to session files. --- src/login/logind-session.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 6c447f5534..b2a496279b 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -175,9 +176,8 @@ int session_save(Session *s) { if (r < 0) goto fail; - assert(s->user); - - fchmod(fileno(f), 0644); + (void) __fsetlocking(f, FSETLOCKING_BYCALLER); + (void) fchmod(fileno(f), 0644); fprintf(f, "# This is private data. Do not parse.\n" From 1007473b49b5aaeef0e53cd4a15f4ed8cf721926 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 3 Aug 2018 20:19:38 +0200 Subject: [PATCH 04/37] units: set StopWhenUnneeded= for the user slice units too We'd like them to go away, just like the user-runtime-dir@.service when they aren't needed anymore. --- units/user-.slice.d/10-defaults.conf | 1 + 1 file changed, 1 insertion(+) diff --git a/units/user-.slice.d/10-defaults.conf b/units/user-.slice.d/10-defaults.conf index f1d118562c..c81a00e050 100644 --- a/units/user-.slice.d/10-defaults.conf +++ b/units/user-.slice.d/10-defaults.conf @@ -11,6 +11,7 @@ Description=User Slice of UID %j Documentation=man:user@.service(5) After=systemd-user-sessions.service +StopWhenUnneeded=yes [Slice] TasksMax=33% From 14df094a51e87013d96ac697ae4f14593cbcad39 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 18:15:07 +0200 Subject: [PATCH 05/37] units: improve Description= string a bit Let's not use the word "wrapper", as it's not clear what that is, and in some way any unit file is a "wrapper"... let's simply say that it's about the runtime directory. --- units/user-runtime-dir@.service.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/units/user-runtime-dir@.service.in b/units/user-runtime-dir@.service.in index 8c5407b881..a8a53f3ead 100644 --- a/units/user-runtime-dir@.service.in +++ b/units/user-runtime-dir@.service.in @@ -8,7 +8,7 @@ # (at your option) any later version. [Unit] -Description=/run/user/%i mount wrapper +Description=User runtime directory /run/user/%i Documentation=man:user@.service(5) After=systemd-user-sessions.service StopWhenUnneeded=yes From b25ba6cf673036e46cbaec77d3c7859ed83d3ca8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 3 Aug 2018 20:20:50 +0200 Subject: [PATCH 06/37] logind: initialize Manager object with structure initialization too --- src/login/logind.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/login/logind.c b/src/login/logind.c index e90c8575dc..e7f3a99b97 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -35,12 +35,14 @@ static int manager_new(Manager **ret) { assert(ret); - m = new0(Manager, 1); + m = new(Manager, 1); if (!m) return -ENOMEM; - m->console_active_fd = -1; - m->reserve_vt_fd = -1; + *m = (Manager) { + .console_active_fd = -1, + .reserve_vt_fd = -1, + }; m->idle_action_not_before_usec = now(CLOCK_MONOTONIC); From 0b6d55cae9b8adc507fbea95d1b2874729a77386 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 3 Aug 2018 20:21:27 +0200 Subject: [PATCH 07/37] logind: improve logging in manager_connect_console() let's make sure we log about every failure Also, complain about systems where /dev/tty0 exists but /sys/class/tty/tty0/active does not. Such systems (usually container environments) are pretty broken as they mount something that is not a VC to /dev/tty0 and they really shouldn't. Systems should either have a VC or not, but not badly fake one by mounting things wildly. This just adds a warning message, as before we'll simply turn off VC handling in this case. --- src/login/logind.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/login/logind.c b/src/login/logind.c index e7f3a99b97..189af504e3 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -789,28 +789,28 @@ static int manager_connect_console(Manager *m) { assert(m); assert(m->console_active_fd < 0); - /* On certain architectures (S390 and Xen, and containers), - /dev/tty0 does not exist, so don't fail if we can't open - it. */ + /* On certain systems (such as S390, Xen, and containers) /dev/tty0 does not exist (as there is no VC), so + * don't fail if we can't open it. */ + if (access("/dev/tty0", F_OK) < 0) return 0; m->console_active_fd = open("/sys/class/tty/tty0/active", O_RDONLY|O_NOCTTY|O_CLOEXEC); if (m->console_active_fd < 0) { - /* On some systems the device node /dev/tty0 may exist - * even though /sys/class/tty/tty0 does not. */ - if (errno == ENOENT) + /* On some systems /dev/tty0 may exist even though /sys/class/tty/tty0 does not. These are broken, but + * common. Let's complain but continue anyway. */ + if (errno == ENOENT) { + log_warning_errno(errno, "System has /dev/tty0 but not /sys/class/tty/tty0/active which is broken, ignoring: %m"); return 0; + } return log_error_errno(errno, "Failed to open /sys/class/tty/tty0/active: %m"); } r = sd_event_add_io(m->event, &m->console_active_event_source, m->console_active_fd, 0, manager_dispatch_console, m); - if (r < 0) { - log_error("Failed to watch foreground console"); - return r; - } + if (r < 0) + return log_error_errno(r, "Failed to watch foreground console: %m"); /* * SIGRTMIN is used as global VT-release signal, SIGRTMIN + 1 is used @@ -829,7 +829,7 @@ static int manager_connect_console(Manager *m) { r = sd_event_add_signal(m->event, NULL, SIGRTMIN, manager_vt_switch, m); if (r < 0) - return r; + return log_error_errno(r, "Failed to subscribe to signal: %m"); return 0; } From d865bc024bf28c17120d7322a81e9a99997a59f6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 18:14:11 +0200 Subject: [PATCH 08/37] logind: save/restore User object's "stopping" field during restarts Whether we are stopping or not is highly relevant, hence don't forget it across restarts. --- src/login/logind-user.c | 20 +++++++++++++++----- src/login/logind-user.h | 5 +++-- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 302d3b08e6..416a2b3aae 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -136,9 +136,11 @@ static int user_save_internal(User *u) { fprintf(f, "# This is private data. Do not parse.\n" "NAME=%s\n" - "STATE=%s\n", + "STATE=%s\n" /* friendly user-facing state */ + "STOPPING=%s\n", /* low-level state */ u->name, - user_state_to_string(user_get_state(u))); + user_state_to_string(user_get_state(u)), + yes_no(u->stopping)); /* LEGACY: no-one reads RUNTIME= anymore, drop it at some point */ if (u->runtime_path) @@ -277,14 +279,14 @@ int user_save(User *u) { } int user_load(User *u) { - _cleanup_free_ char *realtime = NULL, *monotonic = NULL; + _cleanup_free_ char *realtime = NULL, *monotonic = NULL, *stopping = NULL; int r; assert(u); r = parse_env_file(NULL, u->state_file, NEWLINE, "SERVICE_JOB", &u->service_job, - "SLICE_JOB", &u->slice_job, + "STOPPING", &stopping, "REALTIME", &realtime, "MONOTONIC", &monotonic, NULL); @@ -293,12 +295,20 @@ int user_load(User *u) { if (r < 0) return log_error_errno(r, "Failed to read %s: %m", u->state_file); + if (stopping) { + r = parse_boolean(stopping); + if (r < 0) + log_debug_errno(r, "Failed to parse 'STOPPING' boolean: %s", stopping); + else + u->stopping = r; + } + if (realtime) timestamp_deserialize(realtime, &u->timestamp.realtime); if (monotonic) timestamp_deserialize(monotonic, &u->timestamp.monotonic); - return r; + return 0; } static int user_start_service(User *u) { diff --git a/src/login/logind-user.h b/src/login/logind-user.h index eba2325284..03e020b870 100644 --- a/src/login/logind-user.h +++ b/src/login/logind-user.h @@ -36,8 +36,9 @@ struct User { dual_timestamp timestamp; bool in_gc_queue:1; - bool started:1; - bool stopping:1; + + bool started:1; /* Whenever the user being started, has been started or is being stopped again. */ + bool stopping:1; /* Whenever the user is being stopped or has been stopped. */ LIST_HEAD(Session, sessions); LIST_FIELDS(User, gc_queue); From d88ffeeeefda4c3447223fd36f8e30f23c931e48 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 18:19:45 +0200 Subject: [PATCH 09/37] logind: correct bad clean-up path --- src/login/logind-dbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index ed6da4445b..9c7ad1507b 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -846,7 +846,7 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus r = sd_bus_message_enter_container(message, 'a', "(sv)"); if (r < 0) - return r; + goto fail; r = session_start(session, message); if (r < 0) From cce08496e7353e3e9903b42695aba3f9d259b90a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 18:21:37 +0200 Subject: [PATCH 10/37] logind: fix bad error propagation --- src/login/logind-seat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 398c74d712..824bef02a2 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -175,7 +175,7 @@ static int vt_allocate(unsigned int vtnr) { xsprintf(p, "/dev/tty%u", vtnr); fd = open_terminal(p, O_RDWR|O_NOCTTY|O_CLOEXEC); if (fd < 0) - return -errno; + return fd; return 0; } From 190128e407eb24a445554c0e1f956a1d51f97338 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 18:54:03 +0200 Subject: [PATCH 11/37] sd-bus: add new API call sd_bus_error_move() This new call move an sd_bus_error into another one. --- man/rules/meson.build | 1 + man/sd_bus_error.xml | 20 +++++++++++++++++--- src/libsystemd/libsystemd.sym | 2 ++ src/libsystemd/sd-bus/bus-error.c | 22 ++++++++++++++++++++++ src/systemd/sd-bus.h | 1 + 5 files changed, 43 insertions(+), 3 deletions(-) diff --git a/man/rules/meson.build b/man/rules/meson.build index 303b584654..3602bbaa1a 100644 --- a/man/rules/meson.build +++ b/man/rules/meson.build @@ -180,6 +180,7 @@ manpages = [ 'sd_bus_error_get_errno', 'sd_bus_error_has_name', 'sd_bus_error_is_set', + 'sd_bus_error_move', 'sd_bus_error_set', 'sd_bus_error_set_const', 'sd_bus_error_set_errno', diff --git a/man/sd_bus_error.xml b/man/sd_bus_error.xml index 807ca86302..c208f04cb6 100644 --- a/man/sd_bus_error.xml +++ b/man/sd_bus_error.xml @@ -31,6 +31,7 @@ sd_bus_error_set_errnofv sd_bus_error_get_errno sd_bus_error_copy + sd_bus_error_move sd_bus_error_is_set sd_bus_error_has_name @@ -114,6 +115,12 @@ const sd_bus_error *e + + int sd_bus_error_move + sd_bus_error *dst + sd_bus_error *e + + int sd_bus_error_is_set const sd_bus_error *e @@ -245,6 +252,14 @@ Otherwise, they will be copied. Returns a converted errno-like, negative error code. + sd_bus_error_move() is similar to sd_bus_error_copy(), but will + move any error information from e into dst, resetting the + former. This function cannot fail, as no new memory is allocated. Note that if e is not set + (or NULL) dst is initializated to + SD_BUS_ERROR_NULL. Moreover, if dst is NULL no + operation is executed on it and and resources held by e are freed and reset. Returns a + converted errno-like, negative error code. + sd_bus_error_is_set() will return a non-zero value if e is non-NULL and an error has been set, @@ -287,9 +302,8 @@ NULL, and a positive errno value mapped from e->name otherwise. - sd_bus_error_copy() returns 0 or a - positive integer on success, and a negative error value converted - from the error name otherwise. + sd_bus_error_copy() and sd_bus_error_move() return 0 or a positive + integer on success, and a negative error value converted from the error name otherwise. sd_bus_error_is_set() returns a non-zero value when e and the diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index ba682b879a..8d0bebe2ad 100644 --- a/src/libsystemd/libsystemd.sym +++ b/src/libsystemd/libsystemd.sym @@ -577,6 +577,8 @@ global: sd_bus_set_method_call_timeout; sd_bus_get_method_call_timeout; + sd_bus_error_move; + sd_device_ref; sd_device_unref; diff --git a/src/libsystemd/sd-bus/bus-error.c b/src/libsystemd/sd-bus/bus-error.c index 0f79ddc427..e73f7057e1 100644 --- a/src/libsystemd/sd-bus/bus-error.c +++ b/src/libsystemd/sd-bus/bus-error.c @@ -308,6 +308,28 @@ finish: return -bus_error_name_to_errno(e->name); } +_public_ int sd_bus_error_move(sd_bus_error *dest, sd_bus_error *e) { + int r; + + if (!sd_bus_error_is_set(e)) { + + if (dest) + *dest = SD_BUS_ERROR_NULL; + + return 0; + } + + r = -bus_error_name_to_errno(e->name); + + if (dest) { + *dest = *e; + *e = SD_BUS_ERROR_NULL; + } else + sd_bus_error_free(e); + + return r; +} + _public_ int sd_bus_error_set_const(sd_bus_error *e, const char *name, const char *message) { if (!name) return 0; diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 5bc6965916..ce35756861 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -422,6 +422,7 @@ int sd_bus_error_set_errnof(sd_bus_error *e, int error, const char *format, ...) int sd_bus_error_set_errnofv(sd_bus_error *e, int error, const char *format, va_list ap) _sd_printf_(3,0); int sd_bus_error_get_errno(const sd_bus_error *e); int sd_bus_error_copy(sd_bus_error *dest, const sd_bus_error *e); +int sd_bus_error_move(sd_bus_error *dest, sd_bus_error *e); int sd_bus_error_is_set(const sd_bus_error *e); int sd_bus_error_has_name(const sd_bus_error *e, const char *name); From 74fb9617a8d1958d89b4b829a6fe544ac30bb81c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 18:54:51 +0200 Subject: [PATCH 12/37] man: add missing space --- man/sd_bus_error.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/sd_bus_error.xml b/man/sd_bus_error.xml index c208f04cb6..36cdf4c338 100644 --- a/man/sd_bus_error.xml +++ b/man/sd_bus_error.xml @@ -155,7 +155,7 @@ should have both fields initialized to NULL. Set an error structure to SD_BUS_ERROR_NULL in order to reset both fields to NULL. When no longer necessary, resources - held by the sd_bus_errorstructure should + held by the sd_bus_error structure should be destroyed with sd_bus_error_free(). sd_bus_error_set() sets an error From bd26aee1f6bea13fe25b3feb2a5e9cd1be522e7e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 19:00:49 +0200 Subject: [PATCH 13/37] logind: make unit/job active checking more debuggable Let's log the error messages if we get any at debug level. --- src/login/logind-dbus.c | 10 ++++++---- src/login/logind-session.c | 24 ++++++++++++++++++++---- src/login/logind-user.c | 18 ++++++++++++++---- src/login/logind.h | 4 ++-- 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 9c7ad1507b..2e7eb343f1 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -3130,7 +3130,7 @@ int manager_kill_unit(Manager *manager, const char *unit, KillWho who, int signo "ssi", unit, who == KILL_LEADER ? "main" : "all", signo); } -int manager_unit_is_active(Manager *manager, const char *unit) { +int manager_unit_is_active(Manager *manager, const char *unit, sd_bus_error *ret_error) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_free_ char *path = NULL; @@ -3166,17 +3166,18 @@ int manager_unit_is_active(Manager *manager, const char *unit) { sd_bus_error_has_name(&error, BUS_ERROR_LOAD_FAILED)) return false; + sd_bus_error_move(ret_error, &error); return r; } r = sd_bus_message_read(reply, "s", &state); if (r < 0) - return -EINVAL; + return r; - return !streq(state, "inactive") && !streq(state, "failed"); + return !STR_IN_SET(state, "inactive", "failed"); } -int manager_job_is_active(Manager *manager, const char *path) { +int manager_job_is_active(Manager *manager, const char *path, sd_bus_error *ret_error) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; int r; @@ -3201,6 +3202,7 @@ int manager_job_is_active(Manager *manager, const char *path) { if (sd_bus_error_has_name(&error, SD_BUS_ERROR_UNKNOWN_OBJECT)) return false; + sd_bus_error_move(ret_error, &error); return r; } diff --git a/src/login/logind-session.c b/src/login/logind-session.c index b2a496279b..67790f30f9 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -1008,6 +1008,8 @@ static void session_remove_fifo(Session *s) { } bool session_may_gc(Session *s, bool drop_not_started) { + int r; + assert(s); if (drop_not_started && !s->started) @@ -1021,11 +1023,25 @@ bool session_may_gc(Session *s, bool drop_not_started) { return false; } - if (s->scope_job && manager_job_is_active(s->manager, s->scope_job)) - return false; + if (s->scope_job) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - if (s->scope && manager_unit_is_active(s->manager, s->scope)) - return false; + r = manager_job_is_active(s->manager, s->scope_job, &error); + if (r < 0) + log_debug_errno(r, "Failed to determine whether job '%s' is pending, ignoring: %s", s->scope_job, bus_error_message(&error, r)); + if (r != 0) + return false; + } + + if (s->scope) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + + r = manager_unit_is_active(s->manager, s->scope, &error); + if (r < 0) + log_debug_errno(r, "Failed to determine whether unit '%s' is active, ignoring: %s", s->scope, bus_error_message(&error, r)); + if (r != 0) + return false; + } return true; } diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 416a2b3aae..38a45da1dd 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -534,6 +534,8 @@ int user_check_linger_file(User *u) { } bool user_may_gc(User *u, bool drop_not_started) { + int r; + assert(u); if (drop_not_started && !u->started) @@ -545,11 +547,19 @@ bool user_may_gc(User *u, bool drop_not_started) { if (user_check_linger_file(u) > 0) return false; - if (u->slice_job && manager_job_is_active(u->manager, u->slice_job)) - return false; + /* Check if our job is still pending */ + if (u->service_job) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - if (u->service_job && manager_job_is_active(u->manager, u->service_job)) - return false; + r = manager_job_is_active(u->manager, u->service_job, &error); + if (r < 0) + log_debug_errno(r, "Failed to determine whether job '%s' is pending, ignoring: %s", u->service_job, bus_error_message(&error, r)); + if (r != 0) + return false; + } + + /* Note that we don't care if the three units we manage for each user object are up or not, as we are managing + * their state rather than tracking it. */ return true; } diff --git a/src/login/logind.h b/src/login/logind.h index e25bc1e13f..67d12c8b1d 100644 --- a/src/login/logind.h +++ b/src/login/logind.h @@ -166,8 +166,8 @@ int manager_start_unit(Manager *manager, const char *unit, sd_bus_error *error, int manager_stop_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job); int manager_abandon_scope(Manager *manager, const char *scope, sd_bus_error *error); int manager_kill_unit(Manager *manager, const char *unit, KillWho who, int signo, sd_bus_error *error); -int manager_unit_is_active(Manager *manager, const char *unit); -int manager_job_is_active(Manager *manager, const char *path); +int manager_unit_is_active(Manager *manager, const char *unit, sd_bus_error *error); +int manager_job_is_active(Manager *manager, const char *path, sd_bus_error *error); /* gperf lookup function */ const struct ConfigPerfItem* logind_gperf_lookup(const char *key, GPERF_LEN_TYPE length); From 04857cd801022d9f9933efb484c6253572f09870 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 19:02:29 +0200 Subject: [PATCH 14/37] logind: never elect a session that is stopping as display --- src/login/logind-user.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 38a45da1dd..cb0cc30047 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -614,11 +614,10 @@ int user_kill(User *u, int signo) { } static bool elect_display_filter(Session *s) { - /* Return true if the session is a candidate for the user’s ‘primary - * session’ or ‘display’. */ + /* Return true if the session is a candidate for the user’s ‘primary session’ or ‘display’. */ assert(s); - return (s->class == SESSION_USER && !s->stopping); + return s->class == SESSION_USER && s->started && !s->stopping; } static int elect_display_compare(Session *s1, Session *s2) { @@ -664,9 +663,8 @@ void user_elect_display(User *u) { assert(u); - /* This elects a primary session for each user, which we call - * the "display". We try to keep the assignment stable, but we - * "upgrade" to better choices. */ + /* This elects a primary session for each user, which we call the "display". We try to keep the assignment + * stable, but we "upgrade" to better choices. */ log_debug("Electing new display for user %s", u->name); LIST_FOREACH(sessions_by_user, s, u->sessions) { From e555d12635007da7263d0a43ed7307e70a07720d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 19:03:27 +0200 Subject: [PATCH 15/37] logind: make better use of logging functions --- src/login/logind-seat.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 824bef02a2..d67ff116e4 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -199,10 +199,8 @@ int seat_preallocate_vts(Seat *s) { int q; q = vt_allocate(i); - if (q < 0) { - log_error_errno(q, "Failed to preallocate VT %u: %m", i); - r = q; - } + if (q < 0) + r = log_error_errno(q, "Failed to preallocate VT %u: %m", i); } return r; @@ -219,9 +217,9 @@ int seat_apply_acls(Seat *s, Session *old_active) { !!s->active, s->active ? s->active->user->uid : 0); if (r < 0) - log_error_errno(r, "Failed to apply ACLs: %m"); + return log_error_errno(r, "Failed to apply ACLs: %m"); - return r; + return 0; } int seat_set_active(Seat *s, Session *session) { From 75bbdf478c73d78bbe5bdee6f468c2e84a1844c6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 19:04:49 +0200 Subject: [PATCH 16/37] logind: voidify a few calls --- src/login/logind-seat.c | 4 ++-- src/login/logind-session.c | 2 +- src/login/logind-user-dbus.c | 2 +- src/login/logind-user.c | 2 +- src/login/logind.c | 12 ++++++------ 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index d67ff116e4..f0e5aa1988 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -239,7 +239,7 @@ int seat_set_active(Seat *s, Session *session) { session_send_changed(old_active, "Active", NULL); } - seat_apply_acls(s, old_active); + (void) seat_apply_acls(s, old_active); if (session && session->started) { session_send_changed(session, "Active", NULL); @@ -431,7 +431,7 @@ int seat_stop(Seat *s, bool force) { seat_stop_sessions(s, force); - unlink(s->state_file); + (void) unlink(s->state_file); seat_add_to_gc_queue(s); if (s->started) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 67790f30f9..72a99163ef 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -1002,7 +1002,7 @@ static void session_remove_fifo(Session *s) { s->fifo_fd = safe_close(s->fifo_fd); if (s->fifo_path) { - unlink(s->fifo_path); + (void) unlink(s->fifo_path); s->fifo_path = mfree(s->fifo_path); } } diff --git a/src/login/logind-user-dbus.c b/src/login/logind-user-dbus.c index c662a26b9f..9620fb0cfc 100644 --- a/src/login/logind-user-dbus.c +++ b/src/login/logind-user-dbus.c @@ -109,7 +109,7 @@ static int property_get_idle_since_hint( assert(reply); assert(u); - user_get_idle_hint(u, &t); + (void) user_get_idle_hint(u, &t); k = streq(property, "IdleSinceHint") ? t.realtime : t.monotonic; return sd_bus_message_append(reply, "t", k); diff --git a/src/login/logind-user.c b/src/login/logind-user.c index cb0cc30047..763ba58dd4 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -473,7 +473,7 @@ int user_finalize(User *u) { r = k; } - unlink(u->state_file); + (void) unlink(u->state_file); user_add_to_gc_queue(u); if (u->started) { diff --git a/src/login/logind.c b/src/login/logind.c index 189af504e3..734f007674 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -953,13 +953,13 @@ static void manager_gc(Manager *m, bool drop_not_started) { /* First, if we are not closing yet, initiate stopping */ if (session_may_gc(session, drop_not_started) && session_get_state(session) != SESSION_CLOSING) - session_stop(session, false); + (void) session_stop(session, false); /* Normally, this should make the session referenced * again, if it doesn't then let's get rid of it * immediately */ if (session_may_gc(session, drop_not_started)) { - session_finalize(session); + (void) session_finalize(session); session_free(session); } } @@ -970,11 +970,11 @@ static void manager_gc(Manager *m, bool drop_not_started) { /* First step: queue stop jobs */ if (user_may_gc(user, drop_not_started)) - user_stop(user, false); + (void) user_stop(user, false); /* Second step: finalize user */ if (user_may_gc(user, drop_not_started)) { - user_finalize(user); + (void) user_finalize(user); user_free(user); } } @@ -1126,10 +1126,10 @@ static int manager_startup(Manager *m) { /* And start everything */ HASHMAP_FOREACH(seat, m->seats, i) - seat_start(seat); + (void) seat_start(seat); HASHMAP_FOREACH(user, m->users, i) - user_start(user); + (void) user_start(user); HASHMAP_FOREACH(session, m->sessions, i) session_start(session, NULL); From 709d058756da7139181355b63cfad2288eefddfe Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 19:05:26 +0200 Subject: [PATCH 17/37] logind: don't rely on downgrade-to-bool --- src/login/logind-session.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 72a99163ef..a897230397 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -537,7 +537,7 @@ int session_activate(Session *s) { /* on seats with VTs, we let VTs manage session-switching */ if (seat_has_vts(s->seat)) { - if (!s->vtnr) + if (s->vtnr == 0) return -EOPNOTSUPP; return chvt(s->vtnr); From d5ddc930150633f9ce38ed4a6dc9accfd7ceaaac Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 19:05:57 +0200 Subject: [PATCH 18/37] logind: prefer strjoin() over asprintf() --- src/login/logind-session.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index a897230397..32e40bb621 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -960,7 +960,8 @@ int session_create_fifo(Session *s) { if (r < 0) return r; - if (asprintf(&s->fifo_path, "/run/systemd/sessions/%s.ref", s->id) < 0) + s->fifo_path = strjoin("/run/systemd/sessions/", s->id, ".ref"); + if (!s->fifo_path) return -ENOMEM; if (mkfifo(s->fifo_path, 0600) < 0 && errno != EEXIST) @@ -972,7 +973,6 @@ int session_create_fifo(Session *s) { s->fifo_fd = open(s->fifo_path, O_RDONLY|O_CLOEXEC|O_NONBLOCK); if (s->fifo_fd < 0) return -errno; - } if (!s->fifo_event_source) { From 1b88ed3b7db37241323348f640f9adb61c0df47a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 19:34:09 +0200 Subject: [PATCH 19/37] logind: use TAKE_PTR() where we can --- src/login/logind-session-dbus.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c index 25c4981dc0..718f05dd57 100644 --- a/src/login/logind-session-dbus.c +++ b/src/login/logind-session-dbus.c @@ -706,9 +706,7 @@ int session_send_create_reply(Session *s, sd_bus_error *error) { if (!sd_bus_error_is_set(error) && (s->scope_job || s->user->service_job)) return 0; - c = s->create_message; - s->create_message = NULL; - + c = TAKE_PTR(s->create_message); if (error) return sd_bus_reply_method_error(c, error); From b1951bc83ffbbb92ba4de7b9cba845421c2f35b1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 19:34:39 +0200 Subject: [PATCH 20/37] logind: introduce little helper that checks whether a session is ready --- src/login/logind-session-dbus.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c index 718f05dd57..5b09a07ffa 100644 --- a/src/login/logind-session-dbus.c +++ b/src/login/logind-session-dbus.c @@ -689,6 +689,15 @@ int session_send_lock_all(Manager *m, bool lock) { return r; } +static bool session_ready(Session *s) { + assert(s); + + /* Returns true when the session is ready, i.e. all jobs we enqueued for it are done (regardless if successful or not) */ + + return !s->scope_job && + !s->user->service_job; +} + int session_send_create_reply(Session *s, sd_bus_error *error) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *c = NULL; _cleanup_close_ int fifo_fd = -1; @@ -696,14 +705,13 @@ int session_send_create_reply(Session *s, sd_bus_error *error) { assert(s); - /* This is called after the session scope and the user service - * were successfully created, and finishes where + /* This is called after the session scope and the user service were successfully created, and finishes where * bus_manager_create_session() left off. */ if (!s->create_message) return 0; - if (!sd_bus_error_is_set(error) && (s->scope_job || s->user->service_job)) + if (!sd_bus_error_is_set(error) && !session_ready(s)) return 0; c = TAKE_PTR(s->create_message); @@ -714,8 +722,7 @@ int session_send_create_reply(Session *s, sd_bus_error *error) { if (fifo_fd < 0) return fifo_fd; - /* Update the session state file before we notify the client - * about the result. */ + /* Update the session state file before we notify the client about the result. */ session_save(s); p = session_bus_path(s); From e6958b7ea33813b085966ac25817a957c0dad7f9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 19:35:44 +0200 Subject: [PATCH 21/37] logind: propagate session stop errors Let's propagate errors from stopping sessions via seat_stop(). This is similar to how we propagate such errors in user_stop() for all sessions associated with a user. Note that we propagate these errors, but we don't abort the function. --- src/login/logind-seat.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index f0e5aa1988..cc33799230 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -419,7 +419,7 @@ int seat_start(Seat *s) { } int seat_stop(Seat *s, bool force) { - int r = 0; + int r; assert(s); @@ -429,7 +429,7 @@ int seat_stop(Seat *s, bool force) { "SEAT_ID=%s", s->id, LOG_MESSAGE("Removed seat %s.", s->id)); - seat_stop_sessions(s, force); + r = seat_stop_sessions(s, force); (void) unlink(s->state_file); seat_add_to_gc_queue(s); From ea3a7cf6c34163834893f1f4c7af44c8245776ac Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 21:41:54 +0200 Subject: [PATCH 22/37] logind: don't clobber bus error structure if we don't fail --- src/login/logind-dbus.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 2e7eb343f1..599c473d3d 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -3081,7 +3081,8 @@ int manager_stop_unit(Manager *manager, const char *unit, sd_bus_error *error, c return strdup_job(reply, job); } -int manager_abandon_scope(Manager *manager, const char *scope, sd_bus_error *error) { +int manager_abandon_scope(Manager *manager, const char *scope, sd_bus_error *ret_error) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_free_ char *path = NULL; int r; @@ -3098,17 +3099,16 @@ int manager_abandon_scope(Manager *manager, const char *scope, sd_bus_error *err path, "org.freedesktop.systemd1.Scope", "Abandon", - error, + &error, NULL, NULL); if (r < 0) { - if (sd_bus_error_has_name(error, BUS_ERROR_NO_SUCH_UNIT) || - sd_bus_error_has_name(error, BUS_ERROR_LOAD_FAILED) || - sd_bus_error_has_name(error, BUS_ERROR_SCOPE_NOT_RUNNING)) { - sd_bus_error_free(error); + if (sd_bus_error_has_name(&error, BUS_ERROR_NO_SUCH_UNIT) || + sd_bus_error_has_name(&error, BUS_ERROR_LOAD_FAILED) || + sd_bus_error_has_name(&error, BUS_ERROR_SCOPE_NOT_RUNNING)) return 0; - } + sd_bus_error_move(ret_error, &error); return r; } From 25a1ab4ed48b72e974f77a68dcbe3521014787bb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 6 Aug 2018 21:44:45 +0200 Subject: [PATCH 23/37] logind: rework how we manage the slice and user-runtime-dir@.service unit for each user Instead of managing it explicitly, let's simplify things and rely on regular Wants=/Requires= dependencies to pull in these units from user@.service and the session scope, and StopWhenUneeded= to stop these auxiliary units again. This way, they can be pulled in easily by unrelated units too. This simplifies things quite a bit: for each session we now only need to manage the session scope, and for each user the user@.service, the other units are not something we need to manage anymore. This patch also makes sure that if user@.service of a user is masked we will continue to work, and user-runtime-dir@.service will still be correctly pulled in, as it is now a dependency of the scope unit. Fixes: #9461 Replaces: #5546 --- src/login/logind-dbus.c | 58 ++++++++--------- src/login/logind-session.c | 55 ++++++++++------ src/login/logind-session.h | 2 +- src/login/logind-user.c | 128 ++++++++++++++++--------------------- src/login/logind-user.h | 7 +- src/login/logind.c | 2 +- src/login/logind.h | 2 +- 7 files changed, 122 insertions(+), 132 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 599c473d3d..950908e66c 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -848,7 +848,7 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus if (r < 0) goto fail; - r = session_start(session, message); + r = session_start(session, message, error); if (r < 0) goto fail; @@ -2728,24 +2728,20 @@ const sd_bus_vtable manager_vtable[] = { }; static int session_jobs_reply(Session *s, const char *unit, const char *result) { - int r = 0; - assert(s); assert(unit); if (!s->started) - return r; + return 0; - if (streq(result, "done")) - r = session_send_create_reply(s, NULL); - else { + if (result && !streq(result, "done")) { _cleanup_(sd_bus_error_free) sd_bus_error e = SD_BUS_ERROR_NULL; - sd_bus_error_setf(&e, BUS_ERROR_JOB_FAILED, "Start job for unit %s failed with '%s'", unit, result); - r = session_send_create_reply(s, &e); + sd_bus_error_setf(&e, BUS_ERROR_JOB_FAILED, "Start job for unit '%s' failed with '%s'", unit, result); + return session_send_create_reply(s, &e); } - return r; + return session_send_create_reply(s, NULL); } int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *error) { @@ -2778,30 +2774,29 @@ int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *err } session = hashmap_get(m->session_units, unit); - if (session && streq_ptr(path, session->scope_job)) { - session->scope_job = mfree(session->scope_job); - session_jobs_reply(session, unit, result); + if (session) { + if (streq_ptr(path, session->scope_job)) { + session->scope_job = mfree(session->scope_job); + (void) session_jobs_reply(session, unit, result); + + session_save(session); + user_save(session->user); + } - session_save(session); - user_save(session->user); session_add_to_gc_queue(session); } user = hashmap_get(m->user_units, unit); - if (user && - (streq_ptr(path, user->service_job) || - streq_ptr(path, user->slice_job))) { - - if (streq_ptr(path, user->service_job)) + if (user) { + if (streq_ptr(path, user->service_job)) { user->service_job = mfree(user->service_job); - if (streq_ptr(path, user->slice_job)) - user->slice_job = mfree(user->slice_job); + LIST_FOREACH(sessions_by_user, session, user->sessions) + (void) session_jobs_reply(session, unit, NULL /* don't propagate user service failures to the client */); - LIST_FOREACH(sessions_by_user, session, user->sessions) - session_jobs_reply(session, unit, result); + user_save(user); + } - user_save(user); user_add_to_gc_queue(user); } @@ -2933,13 +2928,14 @@ int manager_start_scope( pid_t pid, const char *slice, const char *description, - const char *after, - const char *after2, + char **wants, + char **after, sd_bus_message *more_properties, sd_bus_error *error, char **job) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL, *reply = NULL; + char **i; int r; assert(manager); @@ -2977,14 +2973,14 @@ int manager_start_scope( return r; } - if (!isempty(after)) { - r = sd_bus_message_append(m, "(sv)", "After", "as", 1, after); + STRV_FOREACH(i, wants) { + r = sd_bus_message_append(m, "(sv)", "Wants", "as", 1, *i); if (r < 0) return r; } - if (!isempty(after2)) { - r = sd_bus_message_append(m, "(sv)", "After", "as", 1, after2); + STRV_FOREACH(i, after) { + r = sd_bus_message_append(m, "(sv)", "After", "as", 1, *i); if (r < 0) return r; } diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 32e40bb621..068c4a97a9 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -27,6 +27,7 @@ #include "path-util.h" #include "process-util.h" #include "string-table.h" +#include "strv.h" #include "terminal-util.h" #include "user-util.h" #include "util.h" @@ -560,18 +561,18 @@ int session_activate(Session *s) { return 0; } -static int session_start_scope(Session *s, sd_bus_message *properties) { +static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_error *error) { int r; assert(s); assert(s->user); if (!s->scope) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_free_ char *scope = NULL; - char *job = NULL; const char *description; + s->scope_job = mfree(s->scope_job); + scope = strjoin("session-", s->id, ".scope"); if (!scope) return log_oom(); @@ -584,17 +585,15 @@ static int session_start_scope(Session *s, sd_bus_message *properties) { s->leader, s->user->slice, description, - "systemd-logind.service", - "systemd-user-sessions.service", + STRV_MAKE(s->user->runtime_dir_service, s->user->service), /* These two have StopWhenUnneeded= set, hence add a dep towards them */ + STRV_MAKE("systemd-logind.service", "systemd-user-sessions.service", s->user->runtime_dir_service, s->user->service), /* And order us after some more */ properties, - &error, - &job); + error, + &s->scope_job); if (r < 0) - return log_error_errno(r, "Failed to start session scope %s: %s", scope, bus_error_message(&error, r)); - + return log_error_errno(r, "Failed to start session scope %s: %s", scope, bus_error_message(error, r)); s->scope = TAKE_PTR(scope); - free_and_replace(s->scope_job, job); } if (s->scope) @@ -603,7 +602,7 @@ static int session_start_scope(Session *s, sd_bus_message *properties) { return 0; } -int session_start(Session *s, sd_bus_message *properties) { +int session_start(Session *s, sd_bus_message *properties, sd_bus_error *error) { int r; assert(s); @@ -611,6 +610,9 @@ int session_start(Session *s, sd_bus_message *properties) { if (!s->user) return -ESTALE; + if (s->stopping) + return -EINVAL; + if (s->started) return 0; @@ -618,8 +620,7 @@ int session_start(Session *s, sd_bus_message *properties) { if (r < 0) return r; - /* Create cgroup */ - r = session_start_scope(s, properties); + r = session_start_scope(s, properties, error); if (r < 0) return r; @@ -670,21 +671,24 @@ static int session_stop_scope(Session *s, bool force) { * that is left in the scope is "left-over". Informing systemd about this has the benefit that it will log * when killing any processes left after this point. */ r = manager_abandon_scope(s->manager, s->scope, &error); - if (r < 0) + if (r < 0) { log_warning_errno(r, "Failed to abandon session scope, ignoring: %s", bus_error_message(&error, r)); + sd_bus_error_free(&error); + } + + s->scope_job = mfree(s->scope_job); /* Optionally, let's kill everything that's left now. */ if (force || manager_shall_kill(s->manager, s->user->name)) { - char *job = NULL; - r = manager_stop_unit(s->manager, s->scope, &error, &job); - if (r < 0) - return log_error_errno(r, "Failed to stop session scope: %s", bus_error_message(&error, r)); + r = manager_stop_unit(s->manager, s->scope, &error, &s->scope_job); + if (r < 0) { + if (force) + return log_error_errno(r, "Failed to stop session scope: %s", bus_error_message(&error, r)); - free(s->scope_job); - s->scope_job = job; + log_warning_errno(r, "Failed to stop session scope, ignoring: %s", bus_error_message(&error, r)); + } } else { - s->scope_job = mfree(s->scope_job); /* With no killing, this session is allowed to persist in "closing" state indefinitely. * Therefore session stop and session removal may be two distinct events. @@ -704,8 +708,17 @@ int session_stop(Session *s, bool force) { assert(s); + /* This is called whenever we begin with tearing down a session record. It's called in four cases: explicit API + * request via the bus (either directly for the session object or for the seat or user object this session + * belongs to; 'force' is true), or due to automatic GC (i.e. scope vanished; 'force' is false), or because the + * session FIFO saw an EOF ('force' is false), or because the release timer hit ('force' is false). */ + if (!s->user) return -ESTALE; + if (!s->started) + return 0; + if (s->stopping) + return 0; s->timer_event_source = sd_event_source_unref(s->timer_event_source); diff --git a/src/login/logind-session.h b/src/login/logind-session.h index 572f2545c1..7d17d9a25f 100644 --- a/src/login/logind-session.h +++ b/src/login/logind-session.h @@ -124,7 +124,7 @@ void session_set_idle_hint(Session *s, bool b); int session_get_locked_hint(Session *s); void session_set_locked_hint(Session *s, bool b); int session_create_fifo(Session *s); -int session_start(Session *s, sd_bus_message *properties); +int session_start(Session *s, sd_bus_message *properties, sd_bus_error *error); int session_stop(Session *s, bool force); int session_finalize(Session *s); int session_release(Session *s); diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 763ba58dd4..35189c7d0e 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -68,6 +68,10 @@ int user_new(User **ret, Manager *m, uid_t uid, gid_t gid, const char *name) { if (r < 0) return r; + r = unit_name_build("user-runtime-dir", lu, ".service", &u->runtime_dir_service); + if (r < 0) + return r; + r = hashmap_put(m->users, UID_TO_PTR(uid), u); if (r < 0) return r; @@ -80,6 +84,10 @@ int user_new(User **ret, Manager *m, uid_t uid, gid_t gid, const char *name) { if (r < 0) return r; + r = hashmap_put(m->user_units, u->runtime_dir_service, u); + if (r < 0) + return r; + *ret = TAKE_PTR(u); return 0; } @@ -97,15 +105,18 @@ User *user_free(User *u) { if (u->service) hashmap_remove_value(u->manager->user_units, u->service, u); + if (u->runtime_dir_service) + hashmap_remove_value(u->manager->user_units, u->runtime_dir_service, u); + if (u->slice) hashmap_remove_value(u->manager->user_units, u->slice, u); hashmap_remove_value(u->manager->users, UID_TO_PTR(u->uid), u); - u->slice_job = mfree(u->slice_job); u->service_job = mfree(u->service_job); u->service = mfree(u->service); + u->runtime_dir_service = mfree(u->runtime_dir_service); u->slice = mfree(u->slice); u->runtime_path = mfree(u->runtime_path); u->state_file = mfree(u->state_file); @@ -149,9 +160,6 @@ static int user_save_internal(User *u) { if (u->service_job) fprintf(f, "SERVICE_JOB=%s\n", u->service_job); - if (u->slice_job) - fprintf(f, "SLICE_JOB=%s\n", u->slice_job); - if (u->display) fprintf(f, "DISPLAY=%s\n", u->display->id); @@ -311,65 +319,44 @@ int user_load(User *u) { return 0; } -static int user_start_service(User *u) { +static void user_start_service(User *u) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - char *job; int r; assert(u); + /* Start the service containing the "systemd --user" instance (user@.service). Note that we don't explicitly + * start the per-user slice or the systemd-runtime-dir@.service instance, as those are pulled in both by + * user@.service and the session scopes as dependencies. */ + u->service_job = mfree(u->service_job); - r = manager_start_unit( - u->manager, - u->service, - &error, - &job); + r = manager_start_unit(u->manager, u->service, &error, &u->service_job); if (r < 0) - /* we don't fail due to this, let's try to continue */ - log_error_errno(r, "Failed to start user service, ignoring: %s", bus_error_message(&error, r)); - else - u->service_job = job; - - return 0; + log_warning_errno(r, "Failed to start user service '%s', ignoring: %s", u->service, bus_error_message(&error, r)); } int user_start(User *u) { - int r; - assert(u); if (u->started && !u->stopping) return 0; - /* - * If u->stopping is set, the user is marked for removal and the slice - * and service stop-jobs are queued. We have to clear that flag before - * queing the start-jobs again. If they succeed, the user object can be - * re-used just fine (pid1 takes care of job-ordering and proper - * restart), but if they fail, we want to force another user_stop() so - * possibly pending units are stopped. - * Note that we don't clear u->started, as we have no clue what state - * the user is in on failure here. Hence, we pretend the user is - * running so it will be properly taken down by GC. However, we clearly - * return an error from user_start() in that case, so no further - * reference to the user is taken. - */ + /* If u->stopping is set, the user is marked for removal and service stop-jobs are queued. We have to clear + * that flag before queing the start-jobs again. If they succeed, the user object can be re-used just fine + * (pid1 takes care of job-ordering and proper restart), but if they fail, we want to force another user_stop() + * so possibly pending units are stopped. */ u->stopping = false; if (!u->started) log_debug("Starting services for new user %s.", u->name); - /* Save the user data so far, because pam_systemd will read the - * XDG_RUNTIME_DIR out of it while starting up systemd --user. - * We need to do user_save_internal() because we have not - * "officially" started yet. */ + /* Save the user data so far, because pam_systemd will read the XDG_RUNTIME_DIR out of it while starting up + * systemd --user. We need to do user_save_internal() because we have not "officially" started yet. */ user_save_internal(u); - /* Spawn user systemd */ - r = user_start_service(u); - if (r < 0) - return r; + /* Start user@UID.service */ + user_start_service(u); if (!u->started) { if (!dual_timestamp_is_set(&u->timestamp)) @@ -384,60 +371,50 @@ int user_start(User *u) { return 0; } -static int user_stop_slice(User *u) { +static void user_stop_service(User *u) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - char *job; int r; assert(u); + assert(u->service); - r = manager_stop_unit(u->manager, u->slice, &error, &job); + /* The reverse of user_start_service(). Note that we only stop user@UID.service here, and let StopWhenUnneeded= + * deal with the slice and the user-runtime-dir@.service instance. */ + + u->service_job = mfree(u->service_job); + + r = manager_stop_unit(u->manager, u->service, &error, &u->service_job); if (r < 0) - return log_error_errno(r, "Failed to stop user slice: %s", bus_error_message(&error, r)); - - return free_and_replace(u->slice_job, job); -} - -static int user_stop_service(User *u) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - char *job; - int r; - - assert(u); - - r = manager_stop_unit(u->manager, u->service, &error, &job); - if (r < 0) - return log_error_errno(r, "Failed to stop user service: %s", bus_error_message(&error, r)); - - return free_and_replace(u->service_job, job); + log_warning_errno(r, "Failed to stop user service '%s', ignoring: %s", u->service, bus_error_message(&error, r)); } int user_stop(User *u, bool force) { Session *s; - int r = 0, k; + int r = 0; assert(u); - /* Stop jobs have already been queued */ - if (u->stopping) { + /* This is called whenever we begin with tearing down a user record. It's called in two cases: explicit API + * request to do so via the bus (in which case 'force' is true) and automatically due to GC, if there's no + * session left pinning it (in which case 'force' is false). Note that this just initiates tearing down of the + * user, the User object will remain in memory until user_finalize() is called, see below. */ + + if (!u->started) + return 0; + + if (u->stopping) { /* Stop jobs have already been queued */ user_save(u); - return r; + return 0; } LIST_FOREACH(sessions_by_user, s, u->sessions) { + int k; + k = session_stop(s, force); if (k < 0) r = k; } - /* Kill systemd */ - k = user_stop_service(u); - if (k < 0) - r = k; - - /* Kill cgroup */ - k = user_stop_slice(u); - if (k < 0) - r = k; + user_stop_service(u); u->stopping = true; @@ -452,6 +429,9 @@ int user_finalize(User *u) { assert(u); + /* Called when the user is really ready to be freed, i.e. when all unit stop jobs and suchlike for it are + * done. This is called as a result of an earlier user_done() when all jobs are completed. */ + if (u->started) log_debug("User %s logged out.", u->name); @@ -582,7 +562,7 @@ UserState user_get_state(User *u) { if (u->stopping) return USER_CLOSING; - if (!u->started || u->slice_job || u->service_job) + if (!u->started || u->service_job) return USER_OPENING; if (u->sessions) { diff --git a/src/login/logind-user.h b/src/login/logind-user.h index 03e020b870..5e1f7b813a 100644 --- a/src/login/logind-user.h +++ b/src/login/logind-user.h @@ -25,11 +25,12 @@ struct User { char *name; char *state_file; char *runtime_path; - char *slice; - char *service; + + char *slice; /* user-UID.slice */ + char *service; /* user@UID.service */ + char *runtime_dir_service; /* user-runtime-dir@UID.service */ char *service_job; - char *slice_job; Session *display; diff --git a/src/login/logind.c b/src/login/logind.c index 734f007674..da24b16c0f 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -1132,7 +1132,7 @@ static int manager_startup(Manager *m) { (void) user_start(user); HASHMAP_FOREACH(session, m->sessions, i) - session_start(session, NULL); + (void) session_start(session, NULL, NULL); HASHMAP_FOREACH(inhibitor, m->inhibitors, i) inhibitor_start(inhibitor); diff --git a/src/login/logind.h b/src/login/logind.h index 67d12c8b1d..0e54b5a333 100644 --- a/src/login/logind.h +++ b/src/login/logind.h @@ -161,7 +161,7 @@ int bus_manager_shutdown_or_sleep_now_or_later(Manager *m, const char *unit_name int manager_send_changed(Manager *manager, const char *property, ...) _sentinel_; -int manager_start_scope(Manager *manager, const char *scope, pid_t pid, const char *slice, const char *description, const char *after, const char *after2, sd_bus_message *more_properties, sd_bus_error *error, char **job); +int manager_start_scope(Manager *manager, const char *scope, pid_t pid, const char *slice, const char *description, char **wants, char **after, sd_bus_message *more_properties, sd_bus_error *error, char **job); int manager_start_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job); int manager_stop_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job); int manager_abandon_scope(Manager *manager, const char *scope, sd_bus_error *error); From 061c6607a9f2e39a76ee74048f19b5de16c8fac3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 7 Aug 2018 10:40:50 +0200 Subject: [PATCH 24/37] logind: minor session time handling tweaks --- src/login/logind-session.c | 2 +- src/login/logind-session.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 068c4a97a9..85ee93ca56 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -810,7 +810,7 @@ int session_release(Session *s) { return sd_event_add_time(s->manager->event, &s->timer_event_source, CLOCK_MONOTONIC, - now(CLOCK_MONOTONIC) + RELEASE_USEC, 0, + usec_add(now(CLOCK_MONOTONIC), RELEASE_USEC), 0, release_timeout_callback, s); } diff --git a/src/login/logind-session.h b/src/login/logind-session.h index 7d17d9a25f..fca42b2e6d 100644 --- a/src/login/logind-session.h +++ b/src/login/logind-session.h @@ -97,6 +97,7 @@ struct Session { sd_bus_message *create_message; + /* Set up when a client requested to release the session via the bus */ sd_event_source *timer_event_source; char *controller; From 9afe9efb9340588db553950727a2a9672dc3db24 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 7 Aug 2018 11:02:00 +0200 Subject: [PATCH 25/37] logind: optionally, keep the user@.service instance for eached logged in user around for a while This should speed up rapid logout/login cycles a bit. By default this timeout is now set to 10s. Fixes: #8410 Replaces: #4434 --- man/logind.conf.xml | 11 +++++ src/login/logind-core.c | 2 + src/login/logind-dbus.c | 1 + src/login/logind-gperf.gperf | 1 + src/login/logind-session.c | 4 ++ src/login/logind-user.c | 88 +++++++++++++++++++++++++++++++++--- src/login/logind-user.h | 7 ++- src/login/logind.h | 1 + 8 files changed, 107 insertions(+), 8 deletions(-) diff --git a/man/logind.conf.xml b/man/logind.conf.xml index d3551a1bc2..a407858957 100644 --- a/man/logind.conf.xml +++ b/man/logind.conf.xml @@ -184,6 +184,17 @@ 5. + + UserStopDelaySec= + + Specifies how long to keep the user record and per-user service + user@.service around for a user after they logged out fully. If set to zero, the per-user + service is terminated immediately when the last session of the user has ended. If this option is configured to + non-zero rapid logout/login cycles are sped up, as the user's service manager is not constantly restarted. If + set to infinity the per-user service for a user is never terminated again after first login, + and continues to run until system shutdown. Defaults to 10s. + + HandlePowerKey= HandleSuspendKey= diff --git a/src/login/logind-core.c b/src/login/logind-core.c index 3fe2838fc2..c0d84c05fd 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -29,6 +29,8 @@ void manager_reset_config(Manager *m) { m->reserve_vt = 6; m->remove_ipc = true; m->inhibit_delay_max = 5 * USEC_PER_SEC; + m->user_stop_delay = 10 * USEC_PER_SEC; + m->handle_power_key = HANDLE_POWEROFF; m->handle_suspend_key = HANDLE_SUSPEND; m->handle_hibernate_key = HANDLE_HIBERNATE; diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 950908e66c..b45f4088f2 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2648,6 +2648,7 @@ const sd_bus_vtable manager_vtable[] = { SD_BUS_PROPERTY("BlockInhibited", "s", property_get_inhibited, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("DelayInhibited", "s", property_get_inhibited, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("InhibitDelayMaxUSec", "t", NULL, offsetof(Manager, inhibit_delay_max), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("UserStopDelayUSec", "t", NULL, offsetof(Manager, user_stop_delay), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("HandlePowerKey", "s", property_get_handle_action, offsetof(Manager, handle_power_key), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("HandleSuspendKey", "s", property_get_handle_action, offsetof(Manager, handle_suspend_key), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("HandleHibernateKey", "s", property_get_handle_action, offsetof(Manager, handle_hibernate_key), SD_BUS_VTABLE_PROPERTY_CONST), diff --git a/src/login/logind-gperf.gperf b/src/login/logind-gperf.gperf index c85339dcd3..8829ce7d85 100644 --- a/src/login/logind-gperf.gperf +++ b/src/login/logind-gperf.gperf @@ -23,6 +23,7 @@ Login.KillUserProcesses, config_parse_bool, 0, offse Login.KillOnlyUsers, config_parse_strv, 0, offsetof(Manager, kill_only_users) Login.KillExcludeUsers, config_parse_strv, 0, offsetof(Manager, kill_exclude_users) Login.InhibitDelayMaxSec, config_parse_sec, 0, offsetof(Manager, inhibit_delay_max) +Login.UserStopDelaySec, config_parse_sec, 0, offsetof(Manager, user_stop_delay) Login.HandlePowerKey, config_parse_handle_action, 0, offsetof(Manager, handle_power_key) Login.HandleSuspendKey, config_parse_handle_action, 0, offsetof(Manager, handle_suspend_key) Login.HandleHibernateKey, config_parse_handle_action, 0, offsetof(Manager, handle_hibernate_key) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 85ee93ca56..0592c29dae 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -101,6 +101,8 @@ Session* session_free(Session *s) { if (s->user->display == s) s->user->display = NULL; + + user_update_last_session_timer(s->user); } if (s->seat) { @@ -142,6 +144,8 @@ void session_set_user(Session *s, User *u) { s->user = u; LIST_PREPEND(sessions_by_user, u->sessions, s); + + user_update_last_session_timer(u); } static void session_save_devices(Session *s, FILE *f) { diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 35189c7d0e..4b0dfc7e0a 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -47,6 +47,7 @@ int user_new(User **ret, Manager *m, uid_t uid, gid_t gid, const char *name) { .manager = m, .uid = uid, .gid = gid, + .last_session_timestamp = USEC_INFINITY, }; u->name = strdup(name); @@ -113,6 +114,8 @@ User *user_free(User *u) { hashmap_remove_value(u->manager->users, UID_TO_PTR(u->uid), u); + (void) sd_event_source_unref(u->timer_event_source); + u->service_job = mfree(u->service_job); u->service = mfree(u->service); @@ -170,6 +173,10 @@ static int user_save_internal(User *u) { u->timestamp.realtime, u->timestamp.monotonic); + if (u->last_session_timestamp != USEC_INFINITY) + fprintf(f, "LAST_SESSION_TIMESTAMP=" USEC_FMT "\n", + u->last_session_timestamp); + if (u->sessions) { Session *i; bool first; @@ -287,16 +294,17 @@ int user_save(User *u) { } int user_load(User *u) { - _cleanup_free_ char *realtime = NULL, *monotonic = NULL, *stopping = NULL; + _cleanup_free_ char *realtime = NULL, *monotonic = NULL, *stopping = NULL, *last_session_timestamp = NULL; int r; assert(u); r = parse_env_file(NULL, u->state_file, NEWLINE, - "SERVICE_JOB", &u->service_job, - "STOPPING", &stopping, - "REALTIME", &realtime, - "MONOTONIC", &monotonic, + "SERVICE_JOB", &u->service_job, + "STOPPING", &stopping, + "REALTIME", &realtime, + "MONOTONIC", &monotonic, + "LAST_SESSION_TIMESTAMP", &last_session_timestamp, NULL); if (r == -ENOENT) return 0; @@ -312,9 +320,11 @@ int user_load(User *u) { } if (realtime) - timestamp_deserialize(realtime, &u->timestamp.realtime); + (void) timestamp_deserialize(realtime, &u->timestamp.realtime); if (monotonic) - timestamp_deserialize(monotonic, &u->timestamp.monotonic); + (void) timestamp_deserialize(monotonic, &u->timestamp.monotonic); + if (last_session_timestamp) + (void) timestamp_deserialize(last_session_timestamp, &u->last_session_timestamp); return 0; } @@ -524,6 +534,17 @@ bool user_may_gc(User *u, bool drop_not_started) { if (u->sessions) return false; + if (u->last_session_timestamp != USEC_INFINITY) { + /* All sessions have been closed. Let's see if we shall leave the user record around for a bit */ + + if (u->manager->user_stop_delay == USEC_INFINITY) + return false; /* Leave it around forever! */ + if (u->manager->user_stop_delay > 0 && + now(CLOCK_MONOTONIC) < usec_add(u->last_session_timestamp, u->manager->user_stop_delay)) + return false; /* Leave it around for a bit longer. */ + } + + /* Is this a user that shall stay around forever? */ if (user_check_linger_file(u) > 0) return false; @@ -660,6 +681,59 @@ void user_elect_display(User *u) { } } +static int user_stop_timeout_callback(sd_event_source *es, uint64_t usec, void *userdata) { + User *u = userdata; + + assert(u); + user_add_to_gc_queue(u); + + return 0; +} + +void user_update_last_session_timer(User *u) { + int r; + + assert(u); + + if (u->sessions) { + /* There are sessions, turn off the timer */ + u->last_session_timestamp = USEC_INFINITY; + u->timer_event_source = sd_event_source_unref(u->timer_event_source); + return; + } + + if (u->last_session_timestamp != USEC_INFINITY) + return; /* Timer already started */ + + u->last_session_timestamp = now(CLOCK_MONOTONIC); + + assert(!u->timer_event_source); + + if (u->manager->user_stop_delay == 0 || u->manager->user_stop_delay == USEC_INFINITY) + return; + + if (sd_event_get_state(u->manager->event) == SD_EVENT_FINISHED) { + log_debug("Not allocating user stop timeout, since we are already exiting."); + return; + } + + r = sd_event_add_time(u->manager->event, + &u->timer_event_source, + CLOCK_MONOTONIC, + usec_add(u->last_session_timestamp, u->manager->user_stop_delay), 0, + user_stop_timeout_callback, u); + if (r < 0) + log_warning_errno(r, "Failed to enqueue user stop event source, ignoring: %m"); + + if (DEBUG_LOGGING) { + char s[FORMAT_TIMESPAN_MAX]; + + log_debug("Last session of user '%s' logged out, terminating user context in %s.", + u->name, + format_timespan(s, sizeof(s), u->manager->user_stop_delay, USEC_PER_MSEC)); + } +} + static const char* const user_state_table[_USER_STATE_MAX] = { [USER_OFFLINE] = "offline", [USER_OPENING] = "opening", diff --git a/src/login/logind-user.h b/src/login/logind-user.h index 5e1f7b813a..e05646adc9 100644 --- a/src/login/logind-user.h +++ b/src/login/logind-user.h @@ -34,7 +34,11 @@ struct User { Session *display; - dual_timestamp timestamp; + dual_timestamp timestamp; /* When this User object was 'started' the first time */ + usec_t last_session_timestamp; /* When the number of sessions of this user went from 1 to 0 the last time */ + + /* Set up when the last session of the user logs out */ + sd_event_source *timer_event_source; bool in_gc_queue:1; @@ -62,6 +66,7 @@ int user_load(User *u); int user_kill(User *u, int signo); int user_check_linger_file(User *u); void user_elect_display(User *u); +void user_update_last_session_timer(User *u); extern const sd_bus_vtable user_vtable[]; int user_node_enumerator(sd_bus *bus, const char *path, void *userdata, char ***nodes, sd_bus_error *error); diff --git a/src/login/logind.h b/src/login/logind.h index 0e54b5a333..69de9040ad 100644 --- a/src/login/logind.h +++ b/src/login/logind.h @@ -62,6 +62,7 @@ struct Manager { Hashmap *user_units; usec_t inhibit_delay_max; + usec_t user_stop_delay; /* If an action is currently being executed or is delayed, * this is != 0 and encodes what is being done */ From 238794b15082e6f61d0ce2943d39205289fff7f0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 7 Aug 2018 12:08:24 +0200 Subject: [PATCH 26/37] logind: add hashtable for finding session by leader PID This is useful later on, when we quickly want to find the session for a leader PID. --- src/login/logind-core.c | 15 ++++++++------ src/login/logind-dbus.c | 3 +-- src/login/logind-session.c | 41 +++++++++++++++++++++++++++++++++++--- src/login/logind-session.h | 1 + src/login/logind.c | 4 +++- src/login/logind.h | 1 + 6 files changed, 53 insertions(+), 12 deletions(-) diff --git a/src/login/logind-core.c b/src/login/logind-core.c index c0d84c05fd..f97aa91430 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -339,13 +339,16 @@ int manager_get_session_by_pid(Manager *m, pid_t pid, Session **ret) { if (!pid_is_valid(pid)) return -EINVAL; - r = cg_pid_get_unit(pid, &unit); - if (r < 0) - goto not_found; + s = hashmap_get(m->sessions_by_leader, PID_TO_PTR(pid)); + if (!s) { + r = cg_pid_get_unit(pid, &unit); + if (r < 0) + goto not_found; - s = hashmap_get(m->session_units, unit); - if (!s) - goto not_found; + s = hashmap_get(m->session_units, unit); + if (!s) + goto not_found; + } if (ret) *ret = s; diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index b45f4088f2..518cc935aa 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -782,9 +782,8 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus goto fail; session_set_user(session, user); + session_set_leader(session, leader); - session->leader = leader; - session->audit_id = audit_id; session->type = t; session->class = c; session->remote = remote; diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 0592c29dae..8c363c1197 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -120,6 +120,9 @@ Session* session_free(Session *s) { free(s->scope); } + if (pid_is_valid(s->leader)) + (void) hashmap_remove_value(s->manager->sessions_by_leader, PID_TO_PTR(s->leader), s); + free(s->scope_job); sd_bus_message_unref(s->create_message); @@ -148,6 +151,30 @@ void session_set_user(Session *s, User *u) { user_update_last_session_timer(u); } +int session_set_leader(Session *s, pid_t pid) { + int r; + + assert(s); + + if (!pid_is_valid(pid)) + return -EINVAL; + + if (s->leader == pid) + return 0; + + r = hashmap_put(s->manager->sessions_by_leader, PID_TO_PTR(pid), s); + if (r < 0) + return r; + + if (pid_is_valid(s->leader)) + (void) hashmap_remove_value(s->manager->sessions_by_leader, PID_TO_PTR(s->leader), s); + + s->leader = pid; + (void) audit_session_from_pid(pid, &s->audit_id); + + return 1; +} + static void session_save_devices(Session *s, FILE *f) { SessionDevice *sd; Iterator i; @@ -457,8 +484,16 @@ int session_load(Session *s) { } if (leader) { - if (parse_pid(leader, &s->leader) >= 0) - (void) audit_session_from_pid(s->leader, &s->audit_id); + pid_t pid; + + r = parse_pid(leader, &pid); + if (r < 0) + log_debug_errno(r, "Failed to parse leader PID of session: %s", leader); + else { + r = session_set_leader(s, pid); + if (r < 0) + log_warning_errno(r, "Failed to set session leader PID, ignoring: %m"); + } } if (type) { @@ -893,7 +928,7 @@ int session_get_idle_hint(Session *s, dual_timestamp *t) { /* For sessions with a leader but no explicitly configured * tty, let's check the controlling tty of the leader */ - if (s->leader > 0) { + if (pid_is_valid(s->leader)) { r = get_process_ctty_atime(s->leader, &atime); if (r >= 0) goto found_atime; diff --git a/src/login/logind-session.h b/src/login/logind-session.h index fca42b2e6d..228359c088 100644 --- a/src/login/logind-session.h +++ b/src/login/logind-session.h @@ -116,6 +116,7 @@ Session* session_free(Session *s); DEFINE_TRIVIAL_CLEANUP_FUNC(Session *, session_free); void session_set_user(Session *s, User *u); +int session_set_leader(Session *s, pid_t pid); bool session_may_gc(Session *s, bool drop_not_started); void session_add_to_gc_queue(Session *s); int session_activate(Session *s); diff --git a/src/login/logind.c b/src/login/logind.c index da24b16c0f..acb8bdca45 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -49,6 +49,7 @@ static int manager_new(Manager **ret) { m->devices = hashmap_new(&string_hash_ops); m->seats = hashmap_new(&string_hash_ops); m->sessions = hashmap_new(&string_hash_ops); + m->sessions_by_leader = hashmap_new(NULL); m->users = hashmap_new(NULL); m->inhibitors = hashmap_new(&string_hash_ops); m->buttons = hashmap_new(&string_hash_ops); @@ -56,7 +57,7 @@ static int manager_new(Manager **ret) { m->user_units = hashmap_new(&string_hash_ops); m->session_units = hashmap_new(&string_hash_ops); - if (!m->devices || !m->seats || !m->sessions || !m->users || !m->inhibitors || !m->buttons || !m->user_units || !m->session_units) + if (!m->devices || !m->seats || !m->sessions || !m->sessions_by_leader || !m->users || !m->inhibitors || !m->buttons || !m->user_units || !m->session_units) return -ENOMEM; r = sd_event_default(&m->event); @@ -111,6 +112,7 @@ static Manager* manager_unref(Manager *m) { hashmap_free(m->devices); hashmap_free(m->seats); hashmap_free(m->sessions); + hashmap_free(m->sessions_by_leader); hashmap_free(m->users); hashmap_free(m->inhibitors); hashmap_free(m->buttons); diff --git a/src/login/logind.h b/src/login/logind.h index 69de9040ad..9bdfbdfeee 100644 --- a/src/login/logind.h +++ b/src/login/logind.h @@ -27,6 +27,7 @@ struct Manager { Hashmap *devices; Hashmap *seats; Hashmap *sessions; + Hashmap *sessions_by_leader; Hashmap *users; Hashmap *inhibitors; Hashmap *buttons; From 3d0ef5c7e00155bc74f6f71c34cad518a4ff56ba Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 7 Aug 2018 13:49:34 +0200 Subject: [PATCH 27/37] logind: optionally watch utmp for login data This allows us to determine the TTY an ssh session is for, which is useful to to proper idle detection for ssh sessions. Fixes: #9622 --- src/login/logind-core.c | 143 +++++++++++++++++++++++++++++++++++++ src/login/logind-dbus.c | 5 ++ src/login/logind-session.c | 24 +++++++ src/login/logind-session.h | 14 +++- src/login/logind.c | 10 +++ src/login/logind.h | 8 +++ 6 files changed, 203 insertions(+), 1 deletion(-) diff --git a/src/login/logind-core.c b/src/login/logind-core.c index f97aa91430..bd98ba9b26 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -5,6 +5,9 @@ #include #include #include +#if ENABLE_UTMP +#include +#endif #include "sd-device.h" @@ -17,6 +20,7 @@ #include "fd-util.h" #include "logind.h" #include "parse-util.h" +#include "path-util.h" #include "process-util.h" #include "strv.h" #include "terminal-util.h" @@ -684,3 +688,142 @@ bool manager_all_buttons_ignored(Manager *m) { return true; } + +int manager_read_utmp(Manager *m) { +#if ENABLE_UTMP + int r; + + assert(m); + + if (utmpxname(_PATH_UTMPX) < 0) + return log_error_errno(errno, "Failed to set utmp path to " _PATH_UTMPX ": %m"); + + setutxent(); + + for (;;) { + _cleanup_free_ char *t = NULL; + struct utmpx *u; + const char *c; + Session *s; + + errno = 0; + u = getutxent(); + if (!u) { + if (errno != 0) + log_warning_errno(errno, "Failed to read " _PATH_UTMPX ", ignoring: %m"); + r = 0; + break; + } + + if (u->ut_type != USER_PROCESS) + continue; + + if (!pid_is_valid(u->ut_pid)) + continue; + + t = strndup(u->ut_line, sizeof(u->ut_line)); + if (!t) { + r = log_oom(); + break; + } + + c = path_startswith(t, "/dev/"); + if (c) { + r = free_and_strdup(&t, c); + if (r < 0) { + log_oom(); + break; + } + } + + if (isempty(t)) + continue; + + s = hashmap_get(m->sessions_by_leader, PID_TO_PTR(u->ut_pid)); + if (!s) + continue; + + if (s->tty_validity == TTY_FROM_UTMP && !streq_ptr(s->tty, t)) { + /* This may happen on multiplexed SSH connection (i.e. 'SSH connection sharing'). In + * this case PAM and utmp sessions don't match. In such a case let's invalidate the TTY + * information and never acquire it again. */ + + s->tty = mfree(s->tty); + s->tty_validity = TTY_UTMP_INCONSISTENT; + log_debug("Session '%s' has inconsistent TTY information, dropping TTY information.", s->id); + continue; + } + + /* Never override what we figured out once */ + if (s->tty || s->tty_validity >= 0) + continue; + + s->tty = TAKE_PTR(t); + s->tty_validity = TTY_FROM_UTMP; + log_debug("Acquired TTY information '%s' from utmp for session '%s'.", s->tty, s->id); + } + + endutxent(); + return r; +#else + return 0 +#endif +} + +#if ENABLE_UTMP +static int manager_dispatch_utmp(sd_event_source *s, const struct inotify_event *event, void *userdata) { + Manager *m = userdata; + + assert(m); + + /* If there's indication the file itself might have been removed or became otherwise unavailable, then let's + * reestablish the watch on whatever there's now. */ + if ((event->mask & (IN_ATTRIB|IN_DELETE_SELF|IN_MOVE_SELF|IN_Q_OVERFLOW|IN_UNMOUNT)) != 0) + manager_connect_utmp(m); + + (void) manager_read_utmp(m); + return 0; +} +#endif + +void manager_connect_utmp(Manager *m) { +#if ENABLE_UTMP + sd_event_source *s = NULL; + int r; + + assert(m); + + /* Watch utmp for changes via inotify. We do this to deal with tools such as ssh, which will register the PAM + * session early, and acquire a TTY only much later for the connection. Thus during PAM the TTY won't be known + * yet. ssh will register itself with utmp when it finally acquired the TTY. Hence, let's make use of this, and + * watch utmp for the TTY asynchronously. We use the PAM session's leader PID as key, to find the right entry. + * + * Yes, relying on utmp is pretty ugly, but it's good enough for informational purposes, as well as idle + * detection (which, for tty sessions, relies on the TTY used) */ + + r = sd_event_add_inotify(m->event, &s, _PATH_UTMPX, IN_MODIFY|IN_MOVE_SELF|IN_DELETE_SELF|IN_ATTRIB, manager_dispatch_utmp, m); + if (r < 0) + log_full_errno(r == -ENOENT ? LOG_DEBUG: LOG_WARNING, r, "Failed to create inotify watch on " _PATH_UTMPX ", ignoring: %m"); + else { + r = sd_event_source_set_priority(s, SD_EVENT_PRIORITY_IDLE); + if (r < 0) + log_warning_errno(r, "Failed to adjust utmp event source priority, ignoring: %m"); + + (void) sd_event_source_set_description(s, "utmp"); + } + + sd_event_source_unref(m->utmp_event_source); + m->utmp_event_source = s; +#endif +} + +void manager_reconnect_utmp(Manager *m) { +#if ENABLE_UTMP + assert(m); + + if (m->utmp_event_source) + return; + + manager_connect_utmp(m); +#endif +} diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 518cc935aa..818341db4b 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -773,6 +773,9 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus } while (hashmap_get(m->sessions, id)); } + /* If we are not watching utmp aleady, try again */ + manager_reconnect_utmp(m); + r = manager_add_user_by_uid(m, uid, &user); if (r < 0) goto fail; @@ -795,6 +798,8 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus r = -ENOMEM; goto fail; } + + session->tty_validity = TTY_FROM_PAM; } if (!isempty(display)) { diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 8c363c1197..6a70c9bced 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -56,6 +56,7 @@ int session_new(Session **ret, Manager *m, const char *id) { .fifo_fd = -1, .vtfd = -1, .audit_id = AUDIT_SESSION_INVALID, + .tty_validity = _TTY_VALIDITY_INVALID, }; s->state_file = strappend("/run/systemd/sessions/", id); @@ -246,6 +247,9 @@ int session_save(Session *s) { if (s->tty) fprintf(f, "TTY=%s\n", s->tty); + if (s->tty_validity >= 0) + fprintf(f, "TTY_VALIDITY=%s\n", tty_validity_to_string(s->tty_validity)); + if (s->display) fprintf(f, "DISPLAY=%s\n", s->display); @@ -382,6 +386,7 @@ static int session_load_devices(Session *s, const char *devices) { int session_load(Session *s) { _cleanup_free_ char *remote = NULL, *seat = NULL, + *tty_validity = NULL, *vtnr = NULL, *state = NULL, *position = NULL, @@ -407,6 +412,7 @@ int session_load(Session *s) { "FIFO", &s->fifo_path, "SEAT", &seat, "TTY", &s->tty, + "TTY_VALIDITY", &tty_validity, "DISPLAY", &s->display, "REMOTE_HOST", &s->remote_host, "REMOTE_USER", &s->remote_user, @@ -483,6 +489,16 @@ int session_load(Session *s) { seat_claim_position(s->seat, s, npos); } + if (tty_validity) { + TTYValidity v; + + v = tty_validity_from_string(tty_validity); + if (v < 0) + log_debug("Failed to parse TTY validity: %s", tty_validity); + else + s->tty_validity = v; + } + if (leader) { pid_t pid; @@ -1398,3 +1414,11 @@ static const char* const kill_who_table[_KILL_WHO_MAX] = { }; DEFINE_STRING_TABLE_LOOKUP(kill_who, KillWho); + +static const char* const tty_validity_table[_TTY_VALIDITY_MAX] = { + [TTY_FROM_PAM] = "from-pam", + [TTY_FROM_UTMP] = "from-utmp", + [TTY_UTMP_INCONSISTENT] = "utmp-inconsistent", +}; + +DEFINE_STRING_TABLE_LOOKUP(tty_validity, TTYValidity); diff --git a/src/login/logind-session.h b/src/login/logind-session.h index 228359c088..4f9c967dd9 100644 --- a/src/login/logind-session.h +++ b/src/login/logind-session.h @@ -46,6 +46,14 @@ enum KillWho { _KILL_WHO_INVALID = -1 }; +typedef enum TTYValidity { + TTY_FROM_PAM, + TTY_FROM_UTMP, + TTY_UTMP_INCONSISTENT, /* may happen on ssh sessions with multiplexed TTYs */ + _TTY_VALIDITY_MAX, + _TTY_VALIDITY_INVALID = -1, +} TTYValidity; + struct Session { Manager *manager; @@ -60,8 +68,9 @@ struct Session { dual_timestamp timestamp; - char *tty; char *display; + char *tty; + TTYValidity tty_validity; bool remote; char *remote_user; @@ -160,6 +169,9 @@ 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_; +const char* tty_validity_to_string(TTYValidity t) _const_; +TTYValidity tty_validity_from_string(const char *s) _pure_; + int session_prepare_vt(Session *s); void session_restore_vt(Session *s); void session_leave_vt(Session *s); diff --git a/src/login/logind.c b/src/login/logind.c index acb8bdca45..44899064c5 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -133,6 +133,10 @@ static Manager* manager_unref(Manager *m) { sd_event_source_unref(m->udev_button_event_source); sd_event_source_unref(m->lid_switch_ignore_event_source); +#if ENABLE_UTMP + sd_event_source_unref(m->utmp_event_source); +#endif + safe_close(m->console_active_fd); udev_monitor_unref(m->udev_seat_monitor); @@ -1071,6 +1075,9 @@ static int manager_startup(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to register SIGHUP handler: %m"); + /* Connect to utmp */ + manager_connect_utmp(m); + /* Connect to console */ r = manager_connect_console(m); if (r < 0) @@ -1126,6 +1133,9 @@ static int manager_startup(Manager *m) { /* Reserve the special reserved VT */ manager_reserve_vt(m); + /* Read in utmp if it exists */ + manager_read_utmp(m); + /* And start everything */ HASHMAP_FOREACH(seat, m->seats, i) (void) seat_start(seat); diff --git a/src/login/logind.h b/src/login/logind.h index 9bdfbdfeee..0cccecb5ed 100644 --- a/src/login/logind.h +++ b/src/login/logind.h @@ -44,6 +44,10 @@ struct Manager { sd_event_source *udev_vcsa_event_source; sd_event_source *udev_button_event_source; +#if ENABLE_UTMP + sd_event_source *utmp_event_source; +#endif + int console_active_fd; unsigned n_autovts; @@ -151,6 +155,10 @@ bool manager_is_docked_or_external_displays(Manager *m); bool manager_is_on_external_power(void); bool manager_all_buttons_ignored(Manager *m); +int manager_read_utmp(Manager *m); +void manager_connect_utmp(Manager *m); +void manager_reconnect_utmp(Manager *m); + extern const sd_bus_vtable manager_vtable[]; int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *error); From 07ee5adb356b9fde500c8a5226f24a314789832b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Aug 2018 14:50:57 +0200 Subject: [PATCH 28/37] logind: change user-runtime-dir to query runtime dir size from logind via the bus I think this is a slightly cleaner approach than parsing the configuration file at multiple places, as this way there's only a single reload cycle for logind.conf, and that's systemd-logind.service's runtime. This means that logind and dbus become a requirement of user-runtime-dir, but given that XDG_RUNTIME_DIR is not set anyway without logind and dbus around this isn't really any limitation. This also simplifies linking a bit as this means user-runtime-dir doesn't have to link against any code of logind itself. --- meson.build | 16 ++++++------ src/login/meson.build | 1 - src/login/user-runtime-dir.c | 39 +++++++++++++++++++----------- units/user-runtime-dir@.service.in | 4 +-- 4 files changed, 35 insertions(+), 25 deletions(-) diff --git a/meson.build b/meson.build index 39c427cd26..30834c86e3 100644 --- a/meson.build +++ b/meson.build @@ -1762,15 +1762,15 @@ if conf.get('ENABLE_LOGIND') == 1 args : pam_systemd.full_path()) endif endif -endif -executable('systemd-user-runtime-dir', - user_runtime_dir_sources, - include_directories : includes, - link_with : [libshared, liblogind_core], - install_rpath : rootlibexecdir, - install : true, - install_dir : rootlibexecdir) + executable('systemd-user-runtime-dir', + user_runtime_dir_sources, + include_directories : includes, + link_with : [libshared], + install_rpath : rootlibexecdir, + install : true, + install_dir : rootlibexecdir) +endif if conf.get('HAVE_PAM') == 1 executable('systemd-user-sessions', diff --git a/src/login/meson.build b/src/login/meson.build index 0e1ed18f7a..1cc75fd1cf 100644 --- a/src/login/meson.build +++ b/src/login/meson.build @@ -58,7 +58,6 @@ loginctl_sources = files(''' user_runtime_dir_sources = files(''' user-runtime-dir.c - logind.h '''.split()) if conf.get('ENABLE_LOGIND') == 1 diff --git a/src/login/user-runtime-dir.c b/src/login/user-runtime-dir.c index 69f95a25ab..a149e96b26 100644 --- a/src/login/user-runtime-dir.c +++ b/src/login/user-runtime-dir.c @@ -3,9 +3,11 @@ #include #include +#include "sd-bus.h" + +#include "bus-error.h" #include "fs-util.h" #include "label.h" -#include "logind.h" #include "mkdir.h" #include "mount-util.h" #include "path-util.h" @@ -17,21 +19,28 @@ #include "strv.h" #include "user-util.h" -static int gather_configuration(size_t *runtime_dir_size) { - Manager m = {}; +static int acquire_runtime_dir_size(uint64_t *ret) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_unrefp) sd_bus *bus = NULL; int r; - manager_reset_config(&m); - - r = manager_parse_config_file(&m); + r = sd_bus_default_system(&bus); if (r < 0) - log_warning_errno(r, "Failed to parse logind.conf: %m"); + return log_error_errno(r, "Failed to connect to system bus: %m"); + + r = sd_bus_get_property_trivial(bus, "org.freedesktop.login1", "/org/freedesktop/login1", "org.freedesktop.login1.Manager", "RuntimeDirectorySize", &error, 't', ret); + if (r < 0) + return log_error_errno(r, "Failed to acquire runtime directory size: %s", bus_error_message(&error, r)); - *runtime_dir_size = m.runtime_dir_size; return 0; } -static int user_mkdir_runtime_path(const char *runtime_path, uid_t uid, gid_t gid, size_t runtime_dir_size) { +static int user_mkdir_runtime_path( + const char *runtime_path, + uid_t uid, + gid_t gid, + uint64_t runtime_dir_size) { + int r; assert(runtime_path); @@ -49,10 +58,10 @@ static int user_mkdir_runtime_path(const char *runtime_path, uid_t uid, gid_t gi char options[sizeof("mode=0700,uid=,gid=,size=,smackfsroot=*") + DECIMAL_STR_MAX(uid_t) + DECIMAL_STR_MAX(gid_t) - + DECIMAL_STR_MAX(size_t)]; + + DECIMAL_STR_MAX(uint64_t)]; xsprintf(options, - "mode=0700,uid=" UID_FMT ",gid=" GID_FMT ",size=%zu%s", + "mode=0700,uid=" UID_FMT ",gid=" GID_FMT ",size=%" PRIu64 "%s", uid, gid, runtime_dir_size, mac_smack_use() ? ",smackfsroot=*" : ""); @@ -113,7 +122,7 @@ static int user_remove_runtime_path(const char *runtime_path) { static int do_mount(const char *user) { char runtime_path[sizeof("/run/user") + DECIMAL_STR_MAX(uid_t)]; - size_t runtime_dir_size; + uint64_t runtime_dir_size; uid_t uid; gid_t gid; int r; @@ -126,9 +135,11 @@ static int do_mount(const char *user) { : "Failed to look up user \"%s\": %m", user); - xsprintf(runtime_path, "/run/user/" UID_FMT, uid); + r = acquire_runtime_dir_size(&runtime_dir_size); + if (r < 0) + return r; - assert_se(gather_configuration(&runtime_dir_size) == 0); + xsprintf(runtime_path, "/run/user/" UID_FMT, uid); log_debug("Will mount %s owned by "UID_FMT":"GID_FMT, runtime_path, uid, gid); return user_mkdir_runtime_path(runtime_path, uid, gid, runtime_dir_size); diff --git a/units/user-runtime-dir@.service.in b/units/user-runtime-dir@.service.in index a8a53f3ead..6baa340394 100644 --- a/units/user-runtime-dir@.service.in +++ b/units/user-runtime-dir@.service.in @@ -8,9 +8,9 @@ # (at your option) any later version. [Unit] -Description=User runtime directory /run/user/%i +Description=User Runtime Directory /run/user/%i Documentation=man:user@.service(5) -After=systemd-user-sessions.service +After=systemd-user-sessions.service dbus.service StopWhenUnneeded=yes IgnoreOnIsolate=yes From d5ac9d060267820aabdf9af509a54a1830b27b7d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Aug 2018 15:27:49 +0200 Subject: [PATCH 29/37] logind: add a RequiresMountsFor= dependency from the session scope unit to the home directory of the user This is useful so that during shutdown scope units are always terminated before the mounts necessary for the home directory. (Ideally we'd also add a similar dependency from the user@.service instance to the home directory, but this isn't as easy as that service is defined statically and not dynamically, and hence not easy to modify dynamically, in particular when it comes to deps) --- src/login/logind-core.c | 24 ++++++++++++++++++------ src/login/logind-dbus.c | 7 +++++++ src/login/logind-session.c | 1 + src/login/logind-user.c | 13 ++++++++++++- src/login/logind-user.h | 3 ++- src/login/logind.h | 4 ++-- 6 files changed, 42 insertions(+), 10 deletions(-) diff --git a/src/login/logind-core.c b/src/login/logind-core.c index bd98ba9b26..6354dfe6e6 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -134,7 +134,14 @@ int manager_add_session(Manager *m, const char *id, Session **_session) { return 0; } -int manager_add_user(Manager *m, uid_t uid, gid_t gid, const char *name, User **_user) { +int manager_add_user( + Manager *m, + uid_t uid, + gid_t gid, + const char *name, + const char *home, + User **_user) { + User *u; int r; @@ -143,7 +150,7 @@ int manager_add_user(Manager *m, uid_t uid, gid_t gid, const char *name, User ** u = hashmap_get(m->users, UID_TO_PTR(uid)); if (!u) { - r = user_new(&u, m, uid, gid, name); + r = user_new(&u, m, uid, gid, name, home); if (r < 0) return r; } @@ -154,7 +161,12 @@ int manager_add_user(Manager *m, uid_t uid, gid_t gid, const char *name, User ** return 0; } -int manager_add_user_by_name(Manager *m, const char *name, User **_user) { +int manager_add_user_by_name( + Manager *m, + const char *name, + User **_user) { + + const char *home = NULL; uid_t uid; gid_t gid; int r; @@ -162,11 +174,11 @@ int manager_add_user_by_name(Manager *m, const char *name, User **_user) { assert(m); assert(name); - r = get_user_creds(&name, &uid, &gid, NULL, NULL, 0); + r = get_user_creds(&name, &uid, &gid, &home, NULL, 0); if (r < 0) return r; - return manager_add_user(m, uid, gid, name, _user); + return manager_add_user(m, uid, gid, name, home, _user); } int manager_add_user_by_uid(Manager *m, uid_t uid, User **_user) { @@ -179,7 +191,7 @@ int manager_add_user_by_uid(Manager *m, uid_t uid, User **_user) { if (!p) return errno > 0 ? -errno : -ENOENT; - return manager_add_user(m, uid, p->pw_gid, p->pw_name, _user); + return manager_add_user(m, uid, p->pw_gid, p->pw_name, p->pw_dir, _user); } int manager_add_inhibitor(Manager *m, const char* id, Inhibitor **_inhibitor) { diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 818341db4b..032504e63a 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2935,6 +2935,7 @@ int manager_start_scope( const char *description, char **wants, char **after, + const char *requires_mounts_for, sd_bus_message *more_properties, sd_bus_error *error, char **job) { @@ -2990,6 +2991,12 @@ int manager_start_scope( return r; } + if (!empty_or_root(requires_mounts_for)) { + r = sd_bus_message_append(m, "(sv)", "RequiresMountsFor", "as", 1, requires_mounts_for); + if (r < 0) + return r; + } + /* Make sure that the session shells are terminated with SIGHUP since bash and friends tend to ignore * SIGTERM */ r = sd_bus_message_append(m, "(sv)", "SendSIGHUP", "b", true); diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 6a70c9bced..a1c0a08fda 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -642,6 +642,7 @@ static int session_start_scope(Session *s, sd_bus_message *properties, sd_bus_er description, STRV_MAKE(s->user->runtime_dir_service, s->user->service), /* These two have StopWhenUnneeded= set, hence add a dep towards them */ STRV_MAKE("systemd-logind.service", "systemd-user-sessions.service", s->user->runtime_dir_service, s->user->service), /* And order us after some more */ + s->user->home, properties, error, &s->scope_job); diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 4b0dfc7e0a..e92f2a5243 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -30,7 +30,13 @@ #include "user-util.h" #include "util.h" -int user_new(User **ret, Manager *m, uid_t uid, gid_t gid, const char *name) { +int user_new(User **ret, + Manager *m, + uid_t uid, + gid_t gid, + const char *name, + const char *home) { + _cleanup_(user_freep) User *u = NULL; char lu[DECIMAL_STR_MAX(uid_t) + 1]; int r; @@ -54,6 +60,10 @@ int user_new(User **ret, Manager *m, uid_t uid, gid_t gid, const char *name) { if (!u->name) return -ENOMEM; + u->home = strdup(home); + if (!u->home) + return -ENOMEM; + if (asprintf(&u->state_file, "/run/systemd/users/"UID_FMT, uid) < 0) return -ENOMEM; @@ -124,6 +134,7 @@ User *user_free(User *u) { u->runtime_path = mfree(u->runtime_path); u->state_file = mfree(u->state_file); u->name = mfree(u->name); + u->home = mfree(u->home); return mfree(u); } diff --git a/src/login/logind-user.h b/src/login/logind-user.h index e05646adc9..c41973e27d 100644 --- a/src/login/logind-user.h +++ b/src/login/logind-user.h @@ -23,6 +23,7 @@ struct User { uid_t uid; gid_t gid; char *name; + char *home; char *state_file; char *runtime_path; @@ -49,7 +50,7 @@ struct User { LIST_FIELDS(User, gc_queue); }; -int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name); +int user_new(User **out, Manager *m, uid_t uid, gid_t gid, const char *name, const char *home); User *user_free(User *u); DEFINE_TRIVIAL_CLEANUP_FUNC(User *, user_free); diff --git a/src/login/logind.h b/src/login/logind.h index 0cccecb5ed..b1a7b932eb 100644 --- a/src/login/logind.h +++ b/src/login/logind.h @@ -134,7 +134,7 @@ int manager_add_device(Manager *m, const char *sysfs, bool master, Device **_dev int manager_add_button(Manager *m, const char *name, Button **_button); int manager_add_seat(Manager *m, const char *id, Seat **_seat); int manager_add_session(Manager *m, const char *id, Session **_session); -int manager_add_user(Manager *m, uid_t uid, gid_t gid, const char *name, User **_user); +int manager_add_user(Manager *m, uid_t uid, gid_t gid, const char *name, const char *home, User **_user); int manager_add_user_by_name(Manager *m, const char *name, User **_user); int manager_add_user_by_uid(Manager *m, uid_t uid, User **_user); int manager_add_inhibitor(Manager *m, const char* id, Inhibitor **_inhibitor); @@ -171,7 +171,7 @@ int bus_manager_shutdown_or_sleep_now_or_later(Manager *m, const char *unit_name int manager_send_changed(Manager *manager, const char *property, ...) _sentinel_; -int manager_start_scope(Manager *manager, const char *scope, pid_t pid, const char *slice, const char *description, char **wants, char **after, sd_bus_message *more_properties, sd_bus_error *error, char **job); +int manager_start_scope(Manager *manager, const char *scope, pid_t pid, const char *slice, const char *description, char **wants, char **after, const char *requires_mounts_for, sd_bus_message *more_properties, sd_bus_error *error, char **job); int manager_start_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job); int manager_stop_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job); int manager_abandon_scope(Manager *manager, const char *scope, sd_bus_error *error); From 6996df9b864981980f5b713dc5c7d506a7a4b9bf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Aug 2018 16:03:11 +0200 Subject: [PATCH 30/37] logind: improve error propagation of user_check_linger_file() Let's make this a bit prettier, and propagate unexpected access() errors correctly. (The callers of this function will suppress them, but it's nicer of they do that, rather than us doing that twice in both the callers and the callees) --- src/login/logind-user.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index e92f2a5243..230c8f392d 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -530,8 +530,14 @@ int user_check_linger_file(User *u) { return -ENOMEM; p = strjoina("/var/lib/systemd/linger/", cc); + if (access(p, F_OK) < 0) { + if (errno != ENOENT) + return -errno; - return access(p, F_OK) >= 0; + return false; + } + + return true; } bool user_may_gc(User *u, bool drop_not_started) { From 4e5b605af202c770dfc8e3562d0f8d0440b2fe14 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 8 Aug 2018 16:04:40 +0200 Subject: [PATCH 31/37] logind: automatically GC lingering users for who now user@.service (nor slice, not runtime dir service) is running anymore This heavily borrows from @intelfx' PR #5546, but watches all three units that are associated with a user now: the slice, the user@.service and user-runtime-dir@.service. The logic and reasoning behind it is the same though: there's no value in keeping lingering users around if all their three services are gone. Replaces: #5546 Fixes: #4162 --- src/login/logind-user.c | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index 230c8f392d..16a83ae5d5 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -26,6 +26,7 @@ #include "special.h" #include "stdio-util.h" #include "string-table.h" +#include "strv.h" #include "unit-name.h" #include "user-util.h" #include "util.h" @@ -540,6 +541,27 @@ int user_check_linger_file(User *u) { return true; } +static bool user_unit_active(User *u) { + const char *i; + int r; + + assert(u->service); + assert(u->runtime_dir_service); + assert(u->slice); + + FOREACH_STRING(i, u->service, u->runtime_dir_service, u->slice) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + + r = manager_unit_is_active(u->manager, i, &error); + if (r < 0) + log_debug_errno(r, "Failed to determine whether unit '%s' is active, ignoring: %s", u->service, bus_error_message(&error, r)); + if (r != 0) + return true; + } + + return false; +} + bool user_may_gc(User *u, bool drop_not_started) { int r; @@ -561,8 +583,10 @@ bool user_may_gc(User *u, bool drop_not_started) { return false; /* Leave it around for a bit longer. */ } - /* Is this a user that shall stay around forever? */ - if (user_check_linger_file(u) > 0) + /* Is this a user that shall stay around forever ("linger")? Before we say "no" to GC'ing for lingering users, let's check + * if any of the three units that we maintain for this user is still around. If none of them is, + * there's no need to keep this user around even if lingering is enabled. */ + if (user_check_linger_file(u) > 0 && user_unit_active(u)) return false; /* Check if our job is still pending */ @@ -619,7 +643,7 @@ UserState user_get_state(User *u) { return all_closing ? USER_CLOSING : USER_ONLINE; } - if (user_check_linger_file(u) > 0) + if (user_check_linger_file(u) > 0 && user_unit_active(u)) return USER_LINGERING; return USER_CLOSING; From 2d6718bf3dc552c323c728f2dbaa62ebcb21cbfe Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 12 Sep 2018 19:04:24 +0200 Subject: [PATCH 32/37] units: use =yes rather than =true everywhere So far we always used "yes" instead of "true" in all our unit files, except for one outlier. Let's do this here too. No change in behaviour whatsoever, except that it looks prettier ;-) --- units/user-runtime-dir@.service.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/units/user-runtime-dir@.service.in b/units/user-runtime-dir@.service.in index 6baa340394..c168b89f98 100644 --- a/units/user-runtime-dir@.service.in +++ b/units/user-runtime-dir@.service.in @@ -18,5 +18,5 @@ IgnoreOnIsolate=yes ExecStart=@rootlibexecdir@/systemd-user-runtime-dir start %i ExecStop=@rootlibexecdir@/systemd-user-runtime-dir stop %i Type=oneshot -RemainAfterExit=true +RemainAfterExit=yes Slice=user-%i.slice From 964c4eda5b2c78428db0ad32fda068797db04208 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 25 Sep 2018 12:40:35 +0200 Subject: [PATCH 33/37] man: also use "yes"/"no" rather than "true"/"false" in man pages We usually use yes/no in all our unit files, do the same in the man pages. Triggered by: https://github.com/systemd/systemd/pull/9824#issuecomment-420729987 --- man/systemd-socket-proxyd.xml | 2 +- man/systemd.netdev.xml | 8 ++++---- man/systemd.network.xml | 2 +- man/systemd.offline-updates.xml | 2 +- man/systemd.resource-control.xml | 12 ++++++------ man/systemd.socket.xml | 16 ++++++++-------- man/systemd.xml | 2 +- 7 files changed, 22 insertions(+), 22 deletions(-) diff --git a/man/systemd-socket-proxyd.xml b/man/systemd-socket-proxyd.xml index d48dbc02b6..1869465a3b 100644 --- a/man/systemd-socket-proxyd.xml +++ b/man/systemd-socket-proxyd.xml @@ -52,7 +52,7 @@ socat1. The main differences for systemd-socket-proxyd are support for socket activation with - Accept=false and an event-driven + Accept=no and an event-driven design that scales better with the number of connections. diff --git a/man/systemd.netdev.xml b/man/systemd.netdev.xml index 380d0088ce..971d81aa42 100644 --- a/man/systemd.netdev.xml +++ b/man/systemd.netdev.xml @@ -1470,10 +1470,10 @@ Name=ipip-tun Kind=ipip [Tunnel] -Independent=true +Independent=yes Local=10.65.208.212 Remote=10.65.208.211 -FooOverUDP=true +FooOverUDP=yes FOUDestinationPort=5555 @@ -1484,8 +1484,8 @@ Name=tap-test Kind=tap [Tap] -MultiQueue=true -PacketInfo=true +MultiQueue=yes +PacketInfo=yes /etc/systemd/network/25-sit.netdev diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 500467b67e..def6ad2315 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -666,7 +666,7 @@ An IPv6 address, for which Neighbour Advertisement messages will be proxied. This option may be specified more than once. systemd-networkd will add the entries to the kernel's IPv6 neighbor proxy table. - This option implies but has no effect if + This option implies but has no effect if has been set to false. Defaults to unset. diff --git a/man/systemd.offline-updates.xml b/man/systemd.offline-updates.xml index 113d74a220..13fdfc28de 100644 --- a/man/systemd.offline-updates.xml +++ b/man/systemd.offline-updates.xml @@ -134,7 +134,7 @@ - The update service should declare DefaultDependencies=false, + The update service should declare DefaultDependencies=no, Requires=sysinit.target, After=sysinit.target, After=system-update-pre.target and explicitly pull in any other services it requires. diff --git a/man/systemd.resource-control.xml b/man/systemd.resource-control.xml index 6d5778aa72..08ae2d049a 100644 --- a/man/systemd.resource-control.xml +++ b/man/systemd.resource-control.xml @@ -465,7 +465,7 @@ control group attribute, see cgroup-v2.txt. - Implies IOAccounting=true. + Implies IOAccounting=yes. These settings are supported only if the unified control group hierarchy is used. @@ -760,7 +760,7 @@ the startup phase. Using StartupCPUShares= allows prioritizing specific services at boot-up differently than during normal runtime. - Implies CPUAccounting=true. + Implies CPUAccounting=yes. These settings are deprecated. Use CPUWeight= and StartupCPUWeight= instead. @@ -781,7 +781,7 @@ attribute, see memory.txt. - Implies MemoryAccounting=true. + Implies MemoryAccounting=yes. This setting is deprecated. Use MemoryMax= instead. @@ -822,7 +822,7 @@ boot-up differently than during runtime. Implies - BlockIOAccounting=true. + BlockIOAccounting=yes. These settings are deprecated. Use IOWeight= and StartupIOWeight= instead. @@ -844,7 +844,7 @@ url="https://www.kernel.org/doc/Documentation/cgroup-v1/blkio-controller.txt">blkio-controller.txt. Implies - BlockIOAccounting=true. + BlockIOAccounting=yes. This setting is deprecated. Use IODeviceWeight= instead. @@ -869,7 +869,7 @@ Implies - BlockIOAccounting=true. + BlockIOAccounting=yes. These settings are deprecated. Use IOReadBandwidthMax= and IOWriteBandwidthMax= instead. diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml index 4671c71bc8..72807be7b6 100644 --- a/man/systemd.socket.xml +++ b/man/systemd.socket.xml @@ -68,8 +68,8 @@ or it must be a template unit named the same way. Example: a socket file foo.socket needs a matching service foo.service if - is set. If - is set, a service template file + is set. If + is set, a service template file foo@.service must exist from which services are instantiated for each incoming connection. @@ -395,17 +395,17 @@ incoming traffic. Defaults to . For performance reasons, it is recommended to write new daemons only in a way that is suitable for - . A daemon listening on an + . A daemon listening on an AF_UNIX socket may, but does not need to, call close2 on the received socket before exiting. However, it must not unlink the socket from a file system. It should not invoke shutdown2 - on sockets it got with Accept=false, but it + on sockets it got with Accept=no, but it may do so for sockets it got with - Accept=true set. Setting - Accept=true is mostly useful to allow + Accept=yes set. Setting + Accept=yes is mostly useful to allow daemons designed for usage with inetd8 to work unmodified with systemd socket @@ -429,11 +429,11 @@ MaxConnections= The maximum number of connections to simultaneously run services instances for, when - is set. If more concurrent + is set. If more concurrent connections are coming in, they will be refused until at least one existing connection is terminated. This setting has no effect on sockets configured with - or datagram sockets. Defaults to + or datagram sockets. Defaults to 64. diff --git a/man/systemd.xml b/man/systemd.xml index f12956a8a5..3e464b55dd 100644 --- a/man/systemd.xml +++ b/man/systemd.xml @@ -1080,7 +1080,7 @@ quiet Turn off status output at boot, much like - systemd.show_status=false would. Note that + systemd.show_status=no would. Note that this option is also read by the kernel itself and disables kernel log output. Passing this option hence turns off the usual output from both the system manager and the kernel. From 068981233111fd657639687a1742e5f351d04a05 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 7 Aug 2018 11:01:46 +0200 Subject: [PATCH 34/37] update TODO --- TODO | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/TODO b/TODO index 8e4c120603..e79aeda90e 100644 --- a/TODO +++ b/TODO @@ -25,6 +25,12 @@ Features: * set memory.oom.group in cgroupsv2 for all leaf cgroups +* drop umask() calls and suchlike from our generators, pid1 should set things up correctly anyway + +* paranoia: whenever we process passwords, call mlock() on the memory + first. i.e. look for all places we use string_erase()/string_free_erase() and + augment them with mlock() + * whenever oom_kill memory.event event is triggered print a nice log message * Move RestrictAddressFamily= to the new cgroup create socket @@ -34,6 +40,14 @@ Features: * chown() tty a service is attached to after the service goes down +* replace systemd-reboot.service's ExecStart= with a single SuccessAction= + line, so that we don't need to fork() for executing the reboot + service. Similar for other services like this, such as systemd-exit.service + and so on. Of course, for this to work service units with no ExecYYZ= set but + SuccessAction= set need to be acceptable. + +* optionally: turn on cgroup delegation for per-session scope units + * optionally, if a per-partition GPT flag is set for the root/home/… partitions format the partition on next boot and unset the flag, in order to implement factory reset. also, add a second flag that simply indicates whether such a @@ -41,20 +55,6 @@ Features: show state of these flags, and optionally trigger such a factory reset on next boot by setting the flag. -* logind: maybe watch utmp asynchronously using inotify, and populate our own - tracked session metadata from the fields available therein. Why bother? Right - now, all "ssh" sessions will be tracked without their TTY by logind (which is - not just unfriendly to users as this means "loginctl session-status" shows - less information than "who" in many cases, but also breaks the IdleAction - logic, as we never can detect such sessions as idle, as we have no TTY to - watch). ssh sets the PAM_TTY field on its PAM sessions to "ssh" rather than - the actual pty, because the PAM session is opened early on for new - connections, but the PTY only registered much later (if at all). ssh writes - the utmp record only after a TTY is actually registered, hence we could use - this data then, and use it if it is available. Using utmp for this is ugly of - course, and watching things asynchronously even more so, but it should be - good enough for the idle detection logic at least. - * maybe extend .path units to expose fanotify() per-mount change events * Add a "systemctl list-units --by-slice" mode or so, which rearranges the @@ -472,8 +472,6 @@ Features: * maybe add support for specifier expansion in user.conf, specifically DefaultEnvironment= -* introduce systemd-timesync-wait.service or so to sync on an NTP fix? - * consider showing the unit names during boot up in the status output, not just the unit descriptions * maybe allow timer units with an empty Units= setting, so that they @@ -615,7 +613,6 @@ Features: - document chaining of signal handler for SIGCHLD and child handlers - define more intervals where we will shift wakeup intervals around in, 1h, 6h, 24h, ... - generate a failure of a default event loop is executed out-of-thread - - maybe add support for inotify events (which we can do safely now, with O_PATH) * investigate endianness issues of UUID vs. GUID @@ -674,11 +671,9 @@ Features: * logind: - logind: optionally, ignore idle-hint logic for autosuspend, block suspend as long as a session is around - - When we update the kernel all kind of hibernation should be prohibited until shutdown/reboot - logind: wakelock/opportunistic suspend support - Add pretty name for seats in logind - logind: allow showing logout dialog from system? - - session scopes/user unit: add RequiresMountsFor for the home directory of the user - add Suspend() bus calls which take timestamps to fix double suspend issues when somebody hits suspend and closes laptop quickly. - if pam_systemd is invoked by su from a process that is outside of a any session we should probably just become a NOP, since that's @@ -851,8 +846,6 @@ Features: "machinectl start" with a new --ephemeral switch - "machinectl status" should also show internal logs of the container in question - - "machinectl list-images" should show os-release data, as well as - machine-info data (including deployment level) - "machinectl history" - "machinectl diff" - "machinectl commit" that takes a writable snapshot of a tree, invokes a @@ -1048,8 +1041,6 @@ External: * kernel: add device_type = "fb", "fbcon" to class "graphics" -* drop accountsservice's StandardOutput=syslog and Type=dbus fields - * /usr/bin/service should actually show the new command line * fedora: suggest auto-restart on failure, but not on success and not on coredump. also, ask people to think about changing the start limit logic. Also point people to RestartPreventExitStatus=, SuccessExitStatus= From cf99f8eacf1c864b19a6a02edea78c43f3185cb7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Oct 2018 22:22:52 +0200 Subject: [PATCH 35/37] core: make destructive transaction error a bit more useful --- src/core/transaction.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/transaction.c b/src/core/transaction.c index 045930838b..486c6a4a05 100644 --- a/src/core/transaction.c +++ b/src/core/transaction.c @@ -526,7 +526,9 @@ static int transaction_is_destructive(Transaction *tr, JobMode mode, sd_bus_erro if (j->unit->job && (mode == JOB_FAIL || j->unit->job->irreversible) && job_type_is_conflicting(j->unit->job->type, j->type)) return sd_bus_error_setf(e, BUS_ERROR_TRANSACTION_IS_DESTRUCTIVE, - "Transaction is destructive."); + "Transaction for %s/%s is destructive (%s has '%s' job queued, but '%s' is included in transaction).", + tr->anchor_job->unit->id, job_type_to_string(tr->anchor_job->type), + j->unit->id, job_type_to_string(j->unit->job->type), job_type_to_string(j->type)); } return 0; From 93d4cb09d56e670b0c203dd6ec6939e391a0df59 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Oct 2018 22:23:14 +0200 Subject: [PATCH 36/37] core: fix unfortunate typo in unit_is_unneeded() Follow-up for a3c1168ac293f16d9343d248795bb4c246aaff4a. --- src/core/unit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/unit.c b/src/core/unit.c index 1b9bbf7057..d90456e03c 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -2000,7 +2000,7 @@ bool unit_is_unneeded(Unit *u) { * restart, then don't clean this one up. */ HASHMAP_FOREACH_KEY(v, other, u->dependencies[deps[j]], i) { - if (u->job) + if (other->job) return false; if (!UNIT_IS_INACTIVE_OR_FAILED(unit_active_state(other))) From b92171124819305985ed292cc472f6668a027425 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Oct 2018 22:23:41 +0200 Subject: [PATCH 37/37] logind: validate /run/user/1000 before we set it Let's be safe than sorry, in particular as logind doesn't set it up anymore, but user-runtime-dir@.service does, and logind doesn't really track success of that. --- src/login/pam_systemd.c | 48 +++++++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c index 90ccbd7a83..99c253df4a 100644 --- a/src/login/pam_systemd.c +++ b/src/login/pam_systemd.c @@ -308,6 +308,36 @@ static int update_environment(pam_handle_t *handle, const char *key, const char return r; } +static bool validate_runtime_directory(pam_handle_t *handle, const char *path, uid_t uid) { + struct stat st; + + assert(path); + + /* Just some extra paranoia: let's not set $XDG_RUNTIME_DIR if the directory we'd set it to isn't actually set + * up properly for us. */ + + if (lstat(path, &st) < 0) { + pam_syslog(handle, LOG_ERR, "Failed to stat() runtime directory '%s': %s", path, strerror(errno)); + goto fail; + } + + if (!S_ISDIR(st.st_mode)) { + pam_syslog(handle, LOG_ERR, "Runtime directory '%s' is not actually a directory.", path); + goto fail; + } + + if (st.st_uid != uid) { + pam_syslog(handle, LOG_ERR, "Runtime directory '%s' is not owned by UID " UID_FMT ", as it should.", path, uid); + goto fail; + } + + return true; + +fail: + pam_syslog(handle, LOG_WARNING, "Not setting $XDG_RUNTIME_DIR, as the directory is not in order."); + return false; +} + _public_ PAM_EXTERN int pam_sm_open_session( pam_handle_t *handle, int flags, @@ -367,10 +397,12 @@ _public_ PAM_EXTERN int pam_sm_open_session( if (asprintf(&rt, "/run/user/"UID_FMT, pw->pw_uid) < 0) return PAM_BUF_ERR; - r = pam_misc_setenv(handle, "XDG_RUNTIME_DIR", rt, 0); - if (r != PAM_SUCCESS) { - pam_syslog(handle, LOG_ERR, "Failed to set runtime dir."); - return r; + if (validate_runtime_directory(handle, rt, pw->pw_uid)) { + r = pam_misc_setenv(handle, "XDG_RUNTIME_DIR", rt, 0); + if (r != PAM_SUCCESS) { + pam_syslog(handle, LOG_ERR, "Failed to set runtime dir."); + return r; + } } return PAM_SUCCESS; @@ -574,9 +606,11 @@ _public_ PAM_EXTERN int pam_sm_open_session( * in privileged apps clobbering the runtime directory * unnecessarily. */ - r = update_environment(handle, "XDG_RUNTIME_DIR", runtime_path); - if (r != PAM_SUCCESS) - return r; + if (validate_runtime_directory(handle, runtime_path, pw->pw_uid)) { + r = update_environment(handle, "XDG_RUNTIME_DIR", runtime_path); + if (r != PAM_SUCCESS) + return r; + } } /* Most likely we got the session/type/class from environment variables, but might have gotten the data