From 19cd2b6d20950b0d917987c339ab4dc505746515 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 19 Nov 2024 22:16:25 +0100 Subject: [PATCH 1/2] logind: rework session creation logic, to be more reusable for varlink codepaths This separates the preparatory checks that generate D-Bus errors from the code that actually allocates the session. This make the logic easier to follow and prepares ground so that we can reuse the 2nd part later when exposing session creation via Varlink. --- src/login/logind-dbus.c | 368 +++++++++++++++++++++++----------------- src/login/logind-dbus.h | 17 ++ 2 files changed, 233 insertions(+), 152 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 9ffaf8888c..28bc7b6cef 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -876,194 +876,88 @@ static int manager_choose_session_id( return 0; } -static int create_session( +int manager_create_session( Manager *m, - sd_bus_message *message, - sd_bus_error *error, uid_t uid, - pid_t leader_pid, - int leader_pidfd, + PidRef *leader, /* consumed */ const char *service, - const char *type, - const char *class, + SessionType type, + SessionClass class, const char *desktop, - const char *cseat, - uint32_t vtnr, + Seat *seat, + unsigned vtnr, const char *tty, const char *display, - int remote, + bool remote, const char *remote_user, const char *remote_host, - uint64_t flags) { + Session **ret_session) { - _cleanup_(pidref_done) PidRef leader = PIDREF_NULL; - _cleanup_free_ char *id = NULL; - Session *session = NULL; - User *user = NULL; - Seat *seat = NULL; - SessionType t; - SessionClass c; int r; assert(m); - assert(message); + assert(uid_is_valid(uid)); + assert(pidref_is_set(leader)); + assert(ret_session); - if (!uid_is_valid(uid)) - return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid UID"); + /* Returns: + * -EBUSY → client is already in a session + * -EADDRNOTAVAIL → VT is already taken + * -EUSERS → limit of sessions reached + */ - if (isempty(type)) - t = _SESSION_TYPE_INVALID; - else { - t = session_type_from_string(type); - if (t < 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, - "Invalid session type %s", type); - } - - if (isempty(class)) - c = _SESSION_CLASS_INVALID; - else { - c = session_class_from_string(class); - if (c < 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, - "Invalid session class %s", class); - if (c == SESSION_NONE) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, - "Refusing session class %s", class); - } - - if (flags != 0) - return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Flags must be zero."); - - if (leader_pidfd >= 0) - r = pidref_set_pidfd(&leader, leader_pidfd); - else if (leader_pid == 0) - r = bus_query_sender_pidref(message, &leader); - else { - if (leader_pid < 0) - return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Leader PID is not valid"); - - r = pidref_set_pid(&leader, leader_pid); - } - if (r < 0) - return r; - - if (leader.pid == 1 || pidref_is_self(&leader)) - return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid leader PID"); - - if (isempty(desktop)) - desktop = NULL; - else { - if (!string_is_safe(desktop)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, - "Invalid desktop string %s", desktop); - } - - if (isempty(cseat)) - seat = NULL; - else { - seat = hashmap_get(m->seats, cseat); - if (!seat) - return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_SEAT, - "No seat '%s' known", cseat); - } - - if (tty_is_vc(tty)) { - int v; - - if (!seat) - seat = m->seat0; - else if (seat != m->seat0) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, - "TTY %s is virtual console but seat %s is not seat0", tty, seat->id); - - v = vtnr_from_tty(tty); - 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 == 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"); - - } else if (tty_is_console(tty)) { - - if (!seat) - seat = m->seat0; - else if (seat != m->seat0) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, - "Console TTY specified but seat is not seat0"); - - if (vtnr != 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, - "Console TTY specified but VT number is not 0"); - } - - if (seat) { - if (seat_has_vts(seat)) { - if (!vtnr_is_valid(vtnr)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, - "VT number out of range"); - } else { - if (vtnr != 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, - "Seat has no VTs but VT number not 0"); - } - } - - if (t == _SESSION_TYPE_INVALID) { + if (type == _SESSION_TYPE_INVALID) { if (!isempty(display)) - t = SESSION_X11; + type = SESSION_X11; else if (!isempty(tty)) - t = SESSION_TTY; + type = SESSION_TTY; else - t = SESSION_UNSPECIFIED; + type = SESSION_UNSPECIFIED; } - if (c == _SESSION_CLASS_INVALID) { - if (t == SESSION_UNSPECIFIED) - c = SESSION_BACKGROUND; + if (class == _SESSION_CLASS_INVALID) { + if (type == SESSION_UNSPECIFIED) + class = SESSION_BACKGROUND; else - c = SESSION_USER; + class = SESSION_USER; } /* Check if we are already in a logind session, and if so refuse. */ - r = manager_get_session_by_pidref(m, &leader, /* ret_session= */ NULL); + r = manager_get_session_by_pidref(m, leader, /* ret_session= */ NULL); if (r < 0) return log_debug_errno( r, "Failed to check if process " PID_FMT " is already in a session: %m", - leader.pid); + leader->pid); if (r > 0) - return sd_bus_error_setf(error, BUS_ERROR_SESSION_BUSY, - "Already running in a session or user slice"); + return log_debug_errno(SYNTHETIC_ERRNO(EBUSY), "Client is already in a session."); /* Old gdm and lightdm start the user-session on the same VT as the greeter session. But they destroy * the greeter session after the user-session and want the user-session to take over the VT. We need * to support this for backwards-compatibility, so make sure we allow new sessions on a VT that a * greeter is running on. Furthermore, to allow re-logins, we have to allow a greeter to take over a * used VT for the exact same reasons. */ - if (c != SESSION_GREETER && + if (class != SESSION_GREETER && vtnr > 0 && vtnr < MALLOC_ELEMENTSOF(m->seat0->positions) && m->seat0->positions[vtnr] && m->seat0->positions[vtnr]->class != SESSION_GREETER) - return sd_bus_error_set(error, BUS_ERROR_SESSION_BUSY, "Already occupied by a session"); + return log_debug_errno(SYNTHETIC_ERRNO(EADDRNOTAVAIL), "VT already occupied by a session."); if (hashmap_size(m->sessions) >= m->sessions_max) - return sd_bus_error_setf(error, SD_BUS_ERROR_LIMITS_EXCEEDED, - "Maximum number of sessions (%" PRIu64 ") reached, refusing further sessions.", - m->sessions_max); + return log_debug_errno(SYNTHETIC_ERRNO(EUSERS), "Maximum number of sessions (%" PRIu64 ") reached, refusing further sessions.", m->sessions_max); - r = manager_choose_session_id(m, &leader, &id); + _cleanup_free_ char *id = NULL; + r = manager_choose_session_id(m, leader, &id); if (r < 0) return r; /* If we are not watching utmp already, try again */ manager_reconnect_utmp(m); + User *user = NULL; + Session *session = NULL; + r = manager_add_user_by_uid(m, uid, &user); if (r < 0) goto fail; @@ -1073,21 +967,21 @@ static int create_session( goto fail; session_set_user(session, user); - r = session_set_leader_consume(session, TAKE_PIDREF(leader)); + r = session_set_leader_consume(session, TAKE_PIDREF(*leader)); if (r < 0) goto fail; - session->original_type = session->type = t; + session->original_type = session->type = type; session->remote = remote; session->vtnr = vtnr; - session->class = c; + session->class = class; /* Once the first session that is of a pinning class shows up we'll change the GC mode for the user * from USER_GC_BY_ANY to USER_GC_BY_PIN, so that the user goes away once the last pinning session * goes away. Background: we want that user@.service – when started manually – remains around (which * itself is a non-pinning session), but gets stopped when the last pinning session goes away. */ - if (SESSION_CLASS_PIN_USER(c)) + if (SESSION_CLASS_PIN_USER(class)) user->gc_mode = USER_GC_BY_PIN; if (!isempty(tty)) { @@ -1134,14 +1028,187 @@ static int create_session( goto fail; } + *ret_session = session; + return 0; + +fail: + if (session) + session_add_to_gc_queue(session); + + if (user) + user_add_to_gc_queue(user); + + return r; +} + +static int manager_create_session_by_bus( + Manager *m, + sd_bus_message *message, + sd_bus_error *error, + uid_t uid, + pid_t leader_pid, + int leader_pidfd, + const char *service, + const char *type, + const char *class, + const char *desktop, + const char *cseat, + uint32_t vtnr, + const char *tty, + const char *display, + int remote, + const char *remote_user, + const char *remote_host, + uint64_t flags) { + + int r; + + assert(m); + assert(message); + + if (!uid_is_valid(uid)) + return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid UID"); + + if (flags != 0) + return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Flags must be zero."); + + _cleanup_(pidref_done) PidRef leader = PIDREF_NULL; + if (leader_pidfd >= 0) + r = pidref_set_pidfd(&leader, leader_pidfd); + else if (leader_pid == 0) + r = bus_query_sender_pidref(message, &leader); + else { + if (leader_pid < 0) + return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Leader PID is not valid"); + + r = pidref_set_pid(&leader, leader_pid); + } + if (r < 0) + return r; + + if (leader.pid == 1 || pidref_is_self(&leader)) + return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid leader PID"); + + SessionType t; + if (isempty(type)) + t = _SESSION_TYPE_INVALID; + else { + t = session_type_from_string(type); + if (t < 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Invalid session type %s", type); + } + + SessionClass c; + if (isempty(class)) + c = _SESSION_CLASS_INVALID; + else { + c = session_class_from_string(class); + if (c < 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Invalid session class %s", class); + if (c == SESSION_NONE) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Refusing session class %s", class); + } + + if (isempty(desktop)) + desktop = NULL; + else { + if (!string_is_safe(desktop)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Invalid desktop string %s", desktop); + } + + Seat *seat = NULL; + if (isempty(cseat)) + seat = NULL; + else { + seat = hashmap_get(m->seats, cseat); + if (!seat) + return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_SEAT, + "No seat '%s' known", cseat); + } + + if (isempty(tty)) + tty = NULL; + else if (tty_is_vc(tty)) { + int v; + + if (!seat) + seat = m->seat0; + else if (seat != m->seat0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "TTY %s is virtual console but seat %s is not seat0", tty, seat->id); + + v = vtnr_from_tty(tty); + 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 == 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"); + + } else if (tty_is_console(tty)) { + + if (!seat) + seat = m->seat0; + else if (seat != m->seat0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Console TTY specified but seat is not seat0"); + + if (vtnr != 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Console TTY specified but VT number is not 0"); + } + + if (seat) { + if (seat_has_vts(seat)) { + if (!vtnr_is_valid(vtnr)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "VT number out of range"); + } else { + if (vtnr != 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Seat has no VTs but VT number not 0"); + } + } + + Session *session; + r = manager_create_session( + m, + uid, + &leader, + service, + t, + c, + desktop, + seat, + vtnr, + tty, + display, + remote, + remote_user, + remote_host, + &session); + if (r == -EBUSY) + return sd_bus_error_setf(error, BUS_ERROR_SESSION_BUSY, "Already running in a session or user slice"); + if (r == -EADDRNOTAVAIL) + return sd_bus_error_set(error, BUS_ERROR_SESSION_BUSY, "Virtual terminal already occupied by a session"); + if (r == -EUSERS) + return sd_bus_error_setf(error, SD_BUS_ERROR_LIMITS_EXCEEDED, "Maximum number of sessions (%" PRIu64 ") reached, refusing further sessions.", m->sessions_max); + if (r < 0) + return r; + r = sd_bus_message_enter_container(message, 'a', "(sv)"); if (r < 0) goto fail; - r = session_start(session, message, error); if (r < 0) goto fail; - r = sd_bus_message_exit_container(message); if (r < 0) goto fail; @@ -1163,9 +1230,6 @@ fail: if (session) session_add_to_gc_queue(session); - if (user) - user_add_to_gc_queue(user); - return r; } @@ -1200,7 +1264,7 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus if (r < 0) return r; - return create_session( + return manager_create_session_by_bus( userdata, message, error, @@ -1248,7 +1312,7 @@ static int method_create_session_pidfd(sd_bus_message *message, void *userdata, if (r < 0) return r; - return create_session( + return manager_create_session_by_bus( userdata, message, error, diff --git a/src/login/logind-dbus.h b/src/login/logind-dbus.h index 53e6888f5c..afbf523ede 100644 --- a/src/login/logind-dbus.h +++ b/src/login/logind-dbus.h @@ -47,4 +47,21 @@ int manager_job_is_active(Manager *manager, const char *path, sd_bus_error *erro void manager_load_scheduled_shutdown(Manager *m); +int manager_create_session( + Manager *m, + uid_t uid, + PidRef *leader, + const char *service, + SessionType type, + SessionClass class, + const char *desktop, + Seat *seat, + unsigned vtnr, + const char *tty, + const char *display, + bool remote, + const char *remote_user, + const char *remote_host, + Session **ret_session); + extern const BusObjectImplementation manager_object; From a551f584e2009d5dbb7de8c952778364b9a2e45e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 19 Nov 2024 23:25:31 +0100 Subject: [PATCH 2/2] logind: split create session reply handling in two This prepares ground so that later on we can reply with either D-Bus or Varlink depending on the client's request. --- src/login/logind-session-dbus.c | 46 +++++++++------------------------ src/login/logind-session-dbus.h | 2 +- src/login/logind-session.c | 24 +++++++++++++++++ src/login/logind-session.h | 4 +++ 4 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c index 11c315ade7..2affc32b16 100644 --- a/src/login/logind-session-dbus.c +++ b/src/login/logind-session-dbus.c @@ -899,62 +899,40 @@ int session_send_lock_all(Manager *m, bool lock) { return r; } -static bool session_job_pending(Session *s) { - assert(s); - assert(s->user); - - /* Check if we have some jobs enqueued and not finished yet. Each time we get JobRemoved signal about - * relevant units, session_send_create_reply and hence us is called (see match_job_removed). - * Note that we don't care about job result here. */ - - return s->scope_job || - s->user->runtime_dir_job || - (SESSION_CLASS_WANTS_SERVICE_MANAGER(s->class) && s->user->service_manager_job); -} - -int session_send_create_reply(Session *s, const sd_bus_error *error) { - _cleanup_(sd_bus_message_unrefp) sd_bus_message *c = NULL; - _cleanup_close_ int fifo_fd = -EBADF; - _cleanup_free_ char *p = NULL; - +int session_send_create_reply_bus(Session *s, const sd_bus_error *error) { assert(s); - /* This is called after the session scope and the user service were successfully created, and finishes where - * bus_manager_create_session() left off. */ + /* This is called after the session scope and the user service were successfully created, and + * finishes where manager_create_session() left off. */ - if (!s->create_message) + _cleanup_(sd_bus_message_unrefp) sd_bus_message *c = TAKE_PTR(s->create_message); + if (!c) return 0; - /* If error occurred, return it immediately. Otherwise let's wait for all jobs to finish before - * continuing. */ - if (!sd_bus_error_is_set(error) && session_job_pending(s)) - return 0; - - c = TAKE_PTR(s->create_message); - if (error) + if (sd_bus_error_is_set(error)) return sd_bus_reply_method_error(c, error); - fifo_fd = session_create_fifo(s); + _cleanup_close_ int fifo_fd = session_create_fifo(s); if (fifo_fd < 0) return fifo_fd; /* Update the session state file before we notify the client about the result. */ session_save(s); - p = session_bus_path(s); + _cleanup_free_ char *p = session_bus_path(s); if (!p) return -ENOMEM; - log_debug("Sending reply about created session: " - "id=%s object_path=%s uid=%u runtime_path=%s " + log_debug("Sending D-Bus reply about created session: " + "id=%s object_path=%s uid=" UID_FMT " runtime_path=%s " "session_fd=%d seat=%s vtnr=%u", s->id, p, - (uint32_t) s->user->user_record->uid, + s->user->user_record->uid, s->user->runtime_path, fifo_fd, s->seat ? s->seat->id : "", - (uint32_t) s->vtnr); + s->vtnr); return sd_bus_reply_method_return( c, "soshusub", diff --git a/src/login/logind-session-dbus.h b/src/login/logind-session-dbus.h index 2ff686dbd2..091ad26292 100644 --- a/src/login/logind-session-dbus.h +++ b/src/login/logind-session-dbus.h @@ -15,7 +15,7 @@ int session_send_changed(Session *s, const char *properties, ...) _sentinel_; int session_send_lock(Session *s, bool lock); int session_send_lock_all(Manager *m, bool lock); -int session_send_create_reply(Session *s, const sd_bus_error *error); +int session_send_create_reply_bus(Session *s, const sd_bus_error *error); int session_send_upgrade_reply(Session *s, const sd_bus_error *error); int bus_session_method_activate(sd_bus_message *message, void *userdata, sd_bus_error *error); diff --git a/src/login/logind-session.c b/src/login/logind-session.c index ed93fd7d9e..5d13191af9 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -1648,6 +1648,30 @@ void session_drop_controller(Session *s) { session_restore_vt(s); } +bool session_job_pending(Session *s) { + assert(s); + assert(s->user); + + /* Check if we have some jobs enqueued and not finished yet. Each time we get JobRemoved signal about + * relevant units, session_send_create_reply and hence us is called (see match_job_removed). + * Note that we don't care about job result here. */ + + return s->scope_job || + s->user->runtime_dir_job || + (SESSION_CLASS_WANTS_SERVICE_MANAGER(s->class) && s->user->service_manager_job); +} + +int session_send_create_reply(Session *s, const sd_bus_error *error) { + assert(s); + + /* If error occurred, return it immediately. Otherwise let's wait for all jobs to finish before + * continuing. */ + if (!sd_bus_error_is_set(error) && session_job_pending(s)) + return 0; + + return session_send_create_reply_bus(s, error); +} + static const char* const session_state_table[_SESSION_STATE_MAX] = { [SESSION_OPENING] = "opening", [SESSION_ONLINE] = "online", diff --git a/src/login/logind-session.h b/src/login/logind-session.h index f7ee7a92b6..1f15b57f38 100644 --- a/src/login/logind-session.h +++ b/src/login/logind-session.h @@ -217,6 +217,10 @@ bool session_is_controller(Session *s, const char *sender); int session_set_controller(Session *s, const char *sender, bool force, bool prepare); void session_drop_controller(Session *s); +bool session_job_pending(Session *s); + +int session_send_create_reply(Session *s, const sd_bus_error *error); + static inline bool SESSION_IS_SELF(const char *name) { return isempty(name) || streq(name, "self"); }