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 2c0905e64df008f05dee6b7cd5b5c84588021f0b..fb52c5475e687c70559c6dcf8126784fd11c8641 100644 (file)
--- 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 4c4a6fa6bd5f176d00d118c5161dd8015958ff7d..2dc57686e9ecb7236b18ac6f7e4a144c5ebb6372 100644 (file)
--- 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 0a8663412dda42100e46b43d6eefdd26a7b24782..2a59c4af5233341e389ea3dc73c34e26155842a8 100644 (file)
--- 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");