From: Yu Watanabe Date: Tue, 13 May 2025 14:02:13 +0000 (+0900) Subject: login,udev: avoid race between systemd-logind and systemd-udevd in setting ACLs X-Git-Tag: v258-rc1~625^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c960ca2be1cfd183675df581f049a0c022c1c802;p=thirdparty%2Fsystemd.git login,udev: avoid race between systemd-logind and systemd-udevd in setting ACLs Previously, both udevd and logind modifies ACLs of a device node. Hence, there exists a race something like the following: 1. udevd reads an old state file, 2. logind updates the state file, and apply new ACLs, 3. udevd applies ACLs based on the old state file. This makes logind not update ACLs but trigger uevents for relevant devices to make ACLs updated by udevd. --- diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 2c0905e64df..fb52c5475e6 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -8,12 +8,13 @@ #include "sd-messages.h" #include "alloc-util.h" -#include "devnode-acl.h" +#include "device-util.h" #include "errno-util.h" #include "fd-util.h" #include "fileio.h" #include "format-util.h" #include "fs-util.h" +#include "id128-util.h" #include "logind.h" #include "logind-device.h" #include "logind-seat.h" @@ -78,6 +79,7 @@ Seat* seat_free(Seat *s) { hashmap_remove(s->manager->seats, s->id); + set_free(s->uevents); free(s->positions); free(s->state_file); free(s->id); @@ -207,19 +209,120 @@ int seat_preallocate_vts(Seat *s) { return r; } -int seat_apply_acls(Seat *s, Session *old_active) { +static void seat_triggered_uevents_done(Seat *s) { + assert(s); + + if (!set_isempty(s->uevents)) + return; + + Session *session = s->active; + + if (session) { + session_save(session); + user_save(session->user); + } + + if (session && session->started) { + session_send_changed(session, "Active"); + session_device_resume_all(session); + } + + if (!session || session->started) + seat_send_changed(s, "ActiveSession"); +} + +int manager_process_device_triggered_by_seat(Manager *m, sd_device *dev) { + int r; + + assert(m); + assert(dev); + + sd_id128_t uuid; + r = sd_device_get_trigger_uuid(dev, &uuid); + if (IN_SET(r, -ENOENT, -ENODATA)) + return 0; + if (r < 0) + return r; + + Seat *s; + HASHMAP_FOREACH(s, m->seats) + if (set_contains(s->uevents, &uuid)) + break; + if (!s) + return 0; + + free(ASSERT_PTR(set_remove(s->uevents, &uuid))); + seat_triggered_uevents_done(s); + + const char *id; + r = device_get_seat(dev, &id); + if (r < 0) + return r; + + if (!streq(id, s->id)) { + log_device_debug(dev, "ID_SEAT is changed in the triggered uevent: \"%s\" -> \"%s\"", s->id, id); + return 0; + } + + return 1; /* The uevent is triggered by the relevant seat. */ +} + +static int seat_trigger_devices(Seat *s) { int r; assert(s); - r = devnode_acl_all(s->id, - false, - !!old_active, old_active ? old_active->user->user_record->uid : 0, - !!s->active, s->active ? s->active->user->user_record->uid : 0); + set_clear(s->uevents); + _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; + r = sd_device_enumerator_new(&e); if (r < 0) - return log_error_errno(r, "Failed to apply ACLs: %m"); + return r; + + r = sd_device_enumerator_add_match_tag(e, "uaccess"); + if (r < 0) + return r; + FOREACH_DEVICE(e, d) { + /* Verify that the tag is still in place. */ + r = sd_device_has_current_tag(d, "uaccess"); + if (r < 0) + return r; + if (r == 0) + continue; + + /* In case people mistag devices without nodes, we need to ignore this. */ + r = sd_device_get_devname(d, NULL); + if (r == -ENOENT) + continue; + if (r < 0) + return r; + + const char *id; + r = device_get_seat(d, &id); + if (r < 0) + return r; + + if (!streq(id, s->id)) + continue; + + sd_id128_t uuid; + r = sd_device_trigger_with_uuid(d, SD_DEVICE_CHANGE, &uuid); + if (r < 0) { + log_device_debug_errno(d, r, "Failed to trigger 'change' event, ignoring: %m"); + continue; + } + + _cleanup_free_ sd_id128_t *copy = newdup(sd_id128_t, &uuid, 1); + if (!copy) + return -ENOMEM; + + r = set_ensure_consume(&s->uevents, &id128_hash_ops_free, TAKE_PTR(copy)); + if (r < 0) + return r; + } + + seat_triggered_uevents_done(s); return 0; } @@ -237,7 +340,7 @@ int seat_set_active(Seat *s, Session *session) { * Therefore, if the active session has executed session_leave_vt , * A resume is required here. */ if (session == s->active) { - if (session) { + if (session && set_isempty(s->uevents)) { log_debug("Active session remains unchanged, resuming session devices."); session_device_resume_all(session); } @@ -250,32 +353,13 @@ int seat_set_active(Seat *s, Session *session) { seat_save(s); if (old_active) { + user_save(old_active->user); + session_save(old_active); session_device_pause_all(old_active); session_send_changed(old_active, "Active"); } - (void) seat_apply_acls(s, old_active); - - if (session && session->started) { - session_send_changed(session, "Active"); - session_device_resume_all(session); - } - - if (!session || session->started) - seat_send_changed(s, "ActiveSession"); - - if (session) { - session_save(session); - user_save(session->user); - } - - if (old_active) { - session_save(old_active); - if (!session || session->user != old_active->user) - user_save(old_active->user); - } - - return 0; + return seat_trigger_devices(s); } static Session* seat_get_position(Seat *s, unsigned pos) { diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h index 4c4a6fa6bd5..2dc57686e9e 100644 --- a/src/login/logind-seat.h +++ b/src/login/logind-seat.h @@ -1,6 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include "sd-device.h" + #include "list.h" #include "memory-util.h" #include "string-util.h" @@ -19,6 +21,8 @@ struct Seat { LIST_HEAD(Device, devices); + Set *uevents; + Session *active; Session *pending_switch; LIST_HEAD(Session, sessions); @@ -39,7 +43,8 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Seat*, seat_free); int seat_save(Seat *s); int seat_load(Seat *s); -int seat_apply_acls(Seat *s, Session *old_active); +int manager_process_device_triggered_by_seat(Manager *m, sd_device *dev); + int seat_set_active(Seat *s, Session *session); int seat_switch_to(Seat *s, unsigned num); int seat_switch_to_next(Seat *s); diff --git a/src/login/logind.c b/src/login/logind.c index 0a8663412dd..2a59c4af523 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -614,6 +614,14 @@ static int manager_dispatch_seat_udev(sd_device_monitor *monitor, sd_device *dev assert(device); + /* If the event is triggered by us, do not try to start the relevant seat again. Otherwise, starting + * the seat may trigger uevents again again again... */ + r = manager_process_device_triggered_by_seat(m, device); + if (r < 0) + log_device_warning_errno(device, r, "Failed to process seat device event triggered by us, ignoring: %m"); + if (r != 0) + return 0; + r = manager_process_seat_device(m, device); if (r < 0) log_device_warning_errno(device, r, "Failed to process seat device, ignoring: %m"); @@ -635,6 +643,14 @@ static int manager_dispatch_device_udev(sd_device_monitor *monitor, sd_device *d if (r != 0) return 0; + /* If the event is triggered by us, do not try to start the relevant seat again. Otherwise, starting + * the seat may trigger uevents again again again... */ + r = manager_process_device_triggered_by_seat(m, device); + if (r < 0) + log_device_warning_errno(device, r, "Failed to process seat device event triggered by us, ignoring: %m"); + if (r != 0) + return 0; + r = manager_process_seat_device(m, device); if (r < 0) log_device_warning_errno(device, r, "Failed to process seat device, ignoring: %m");