From 14ef4a6506bd37b7d7acfda216d399d5f156819a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 21 Mar 2018 19:04:45 +0100 Subject: [PATCH 1/3] logind: rework manager_get_{user|session}_by_pid() a bit Let's make sure we always initialize the return value if we return non-negative. Just a matter of coding style: we should always initialize our return values when we return >= 0, and leave them unclobbered if we return < 0. --- src/login/logind-core.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/login/logind-core.c b/src/login/logind-core.c index 41b4d4d8d7..8c022cf0f1 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -282,7 +282,7 @@ int manager_process_button_device(Manager *m, struct udev_device *d) { return 0; } -int manager_get_session_by_pid(Manager *m, pid_t pid, Session **session) { +int manager_get_session_by_pid(Manager *m, pid_t pid, Session **ret) { _cleanup_free_ char *unit = NULL; Session *s; int r; @@ -294,38 +294,51 @@ int manager_get_session_by_pid(Manager *m, pid_t pid, Session **session) { r = cg_pid_get_unit(pid, &unit); if (r < 0) - return 0; + goto not_found; s = hashmap_get(m->session_units, unit); if (!s) - return 0; + goto not_found; + + if (ret) + *ret = s; - if (session) - *session = s; return 1; + +not_found: + if (ret) + *ret = NULL; + return 0; } -int manager_get_user_by_pid(Manager *m, pid_t pid, User **user) { +int manager_get_user_by_pid(Manager *m, pid_t pid, User **ret) { _cleanup_free_ char *unit = NULL; User *u; int r; assert(m); - assert(user); if (!pid_is_valid(pid)) return -EINVAL; r = cg_pid_get_slice(pid, &unit); if (r < 0) - return 0; + goto not_found; u = hashmap_get(m->user_units, unit); if (!u) - return 0; + goto not_found; + + if (ret) + *ret = u; - *user = u; return 1; + +not_found: + if (ret) + *ret = NULL; + + return 0; } int manager_get_idle_hint(Manager *m, dual_timestamp *t) { From bc2b6332a2b07644fd15e1a9e0b8678660c9782a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 21 Mar 2018 19:06:21 +0100 Subject: [PATCH 2/3] logind: use manager_get_user_by_pid() where appropriate The current code reimplemented something like the manager_get_user_by_pid() logic on its own, manually. Let's unify this. --- src/login/logind-dbus.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index ef5c478fef..2fa034fe3b 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -658,10 +658,9 @@ static int method_list_inhibitors(sd_bus_message *message, void *userdata, sd_bu static int method_create_session(sd_bus_message *message, void *userdata, sd_bus_error *error) { const char *service, *type, *class, *cseat, *tty, *display, *remote_user, *remote_host, *desktop; - uint32_t audit_id = 0; - _cleanup_free_ char *unit = NULL; - _cleanup_free_ char *id = NULL; + _cleanup_free_ char *unit = NULL, *id = NULL; Session *session = NULL; + uint32_t audit_id = 0; Manager *m = userdata; User *user = NULL; Seat *seat = NULL; @@ -685,7 +684,7 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus if (!uid_is_valid(uid)) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid UID"); - if (leader < 0 || leader == 1) + if (leader < 0 || leader == 1 || leader == getpid_cached()) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid leader PID"); if (isempty(type)) @@ -731,7 +730,7 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus if (v <= 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Cannot determine VT number from virtual console TTY %s", tty); - if (!vtnr) + if (vtnr == 0) vtnr = (uint32_t) v; else if (vtnr != (uint32_t) v) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Specified TTY and VT number do not match"); @@ -749,7 +748,7 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus if (seat) { if (seat_has_vts(seat)) { - if (!vtnr || vtnr > 63) + if (vtnr <= 0 || vtnr > 63) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "VT number out of range"); } else { if (vtnr != 0) @@ -789,16 +788,13 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus return r; } - /* - * Check if we are already in a logind session. Or if we are in user@.service - * which is a special PAM session that avoids creating a logind session. - */ - r = cg_pid_get_unit(leader, &unit); + /* Check if we are already in a logind session. Or if we are in user@.service which is a special PAM session + * that avoids creating a logind session. */ + r = manager_get_user_by_pid(m, leader, NULL); if (r < 0) return r; - if (hashmap_get(m->session_units, unit) || - hashmap_get(m->user_units, unit)) - return sd_bus_error_setf(error, BUS_ERROR_SESSION_BUSY, "Already running in a session"); + if (r > 0) + return sd_bus_error_setf(error, BUS_ERROR_SESSION_BUSY, "Already running in a session or user slice"); /* * Old gdm and lightdm start the user-session on the same VT as @@ -832,9 +828,8 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus * the audit data and let's better register a new * ID */ if (hashmap_get(m->sessions, id)) { - log_warning("Existing logind session ID %s used by new audit session, ignoring", id); + log_warning("Existing logind session ID %s used by new audit session, ignoring.", id); audit_id = AUDIT_SESSION_INVALID; - id = mfree(id); } } @@ -926,8 +921,7 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus session->create_message = sd_bus_message_ref(message); - /* Now, let's wait until the slice unit and stuff got - * created. We send the reply back from + /* Now, let's wait until the slice unit and stuff got created. We send the reply back from * session_send_create_reply(). */ return 1; From e743bca2a606a78655c7ca1f5b9e6409e02627ae Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 21 Mar 2018 19:06:59 +0100 Subject: [PATCH 3/3] logind: drop obsolete comment The code matching this comment was removed in a50df72b37ce2a7caf7775c70d18c3f9504b9e80 in 2014, let's drop the comment too. --- src/login/logind-dbus.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 2fa034fe3b..22116aa236 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -3088,13 +3088,8 @@ int manager_start_scope( return r; } - /* cgroup empty notification is not available in containers - * currently. To make this less problematic, let's shorten the - * stop timeout for sessions, so that we don't wait - * forever. */ - - /* Make sure that the session shells are terminated with - * SIGHUP since bash and friends tend to ignore SIGTERM */ + /* 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); if (r < 0) return r;