logind: cancel long-press timers when the Button is freed

The four *_long_press_event_source timers are Manager-scoped but armed
by start_long_press() with the specific Button that started the press as
their userdata. button_free() unrefs only the Button's own io and check
event sources, never these timers. If the input device backing the
Button is unplugged while a long press is in progress,
manager_process_button_device()'s REMOVE branch calls button_free() and
the still-armed timer fires up to LONG_PRESS_DURATION (5s) later,
dereferencing the freed Button as userdata (b->manager, b->seat).

Cancel each long-press timer the Button owns (matched by userdata) in
button_free().
Also unref all four sources in manager_free() instead of just the reboot
one, for symmetry at daemon teardown.

Follow-up for 952b26c75dae33a322b76bffcff287ff187ef9b4

diff --git a/src/login/logind-button.c b/src/login/logind-button.c
index 285927a14d6256c1453378d77cff80acb2501be9..b7fca7442a2e6e70e3fbcbd60b6f373d5f71d478 100644 (file)
--- a/src/login/logind-button.c
+++ b/src/login/logind-button.c
@@ -88,12 +88,28 @@ Button* button_new(Manager *m, const char *name) {
         return b;
 }
 
+static void stop_long_press(Button *b, sd_event_source **e) {
+        assert(b);
+        assert(e);
+
+        /* The long press timers are Manager-scoped but armed with the Button that started the
+         * press as userdata (see start_long_press()). Cancel the one this Button owns before it is
+         * freed. */
+        if (*e && sd_event_source_get_userdata(*e) == b)
+                *e = sd_event_source_unref(*e);
+}
+
 Button *button_free(Button *b) {
         if (!b)
                 return NULL;
 
         hashmap_remove(b->manager->buttons, b->name);
 
+        stop_long_press(b, &b->manager->power_key_long_press_event_source);
+        stop_long_press(b, &b->manager->reboot_key_long_press_event_source);
+        stop_long_press(b, &b->manager->suspend_key_long_press_event_source);
+        stop_long_press(b, &b->manager->hibernate_key_long_press_event_source);
+
         sd_event_source_unref(b->io_event_source);
         sd_event_source_unref(b->check_event_source);
 

diff --git a/src/login/logind.c b/src/login/logind.c
index 973506a8922a143188379db340dab397389d25e2..fb8e841b9d4b1031f4d653b339602777caf1c2fd 100644 (file)
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -141,7 +141,10 @@ static Manager* manager_free(Manager *m) {
         sd_event_source_unref(m->console_active_event_source);
         sd_event_source_unref(m->lid_switch_ignore_event_source);
 
+        sd_event_source_unref(m->power_key_long_press_event_source);
         sd_event_source_unref(m->reboot_key_long_press_event_source);
+        sd_event_source_unref(m->suspend_key_long_press_event_source);
+        sd_event_source_unref(m->hibernate_key_long_press_event_source);
 
 #if ENABLE_UTMP
         sd_event_source_unref(m->utmp_event_source);

diff --git a/src/login/test-login-tables.c b/src/login/test-login-tables.c
index fcba4334ea63c15b83ad220d41e1116d968873d3..2c064eb9eb4ac4ab8322b2d0bd4c2631e66f347a 100644 (file)
--- a/src/login/test-login-tables.c
+++ b/src/login/test-login-tables.c
@@ -1,11 +1,19 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
+#include "sd-event.h"
+
+#include "alloc-util.h"
+#include "hash-funcs.h"
+#include "hashmap.h"
+#include "logind.h"
 #include "logind-action.h"
+#include "logind-button.h"
 #include "logind-session.h"
 #include "logind-user.h"
 #include "sleep-config.h"
 #include "test-tables.h"
 #include "tests.h"
+#include "time-util.h"
 
 static void test_sleep_handle_action(void) {
         for (HandleAction action = _HANDLE_ACTION_SLEEP_FIRST; action < _HANDLE_ACTION_SLEEP_LAST; action++) {
@@ -21,6 +29,46 @@ static void test_sleep_handle_action(void) {
         }
 }
 
+static int dummy_time_handler(sd_event_source *s, uint64_t usec, void *userdata) {
+        return 0;
+}
+
+/* Verify that button_free() cancels the Manager-scoped long-press timer the Button armed, and at the same
+ * time leaves a timer armed by a different Button alone. */
+static void test_button_free_cancels_long_press(void) {
+        _cleanup_(sd_event_unrefp) sd_event *e = NULL;
+        _cleanup_(hashmap_freep) Hashmap *buttons = NULL;
+        _cleanup_free_ Manager *m = NULL;
+        Button *b, *b2;
+
+        assert_se(m = new0(Manager, 1));
+        assert_se(sd_event_new(&e) >= 0);
+        assert_se(buttons = hashmap_new(&string_hash_ops));
+        m->event = e;
+        m->buttons = buttons;
+
+        assert_se(b = button_new(m, "test-button"));
+        assert_se(b2 = button_new(m, "other-button"));
+
+        /* Arm a long-press timer owned by b, exactly as start_long_press() does: the source lives in
+         * a Manager-scoped field and carries the Button as userdata. */
+        assert_se(sd_event_add_time_relative(e, &m->power_key_long_press_event_source, CLOCK_MONOTONIC,
+                                             10 * USEC_PER_SEC, 0, dummy_time_handler, b) >= 0);
+        /* ... and another one owned by b2. */
+        assert_se(sd_event_add_time_relative(e, &m->reboot_key_long_press_event_source, CLOCK_MONOTONIC,
+                                             10 * USEC_PER_SEC, 0, dummy_time_handler, b2) >= 0);
+
+        /* Freeing b must cancel the timer b armed. */
+        button_free(b);
+        assert_se(!m->power_key_long_press_event_source);
+
+        /* ... but freeing b must not touch a timer owned by another Button. */
+        assert_se(m->reboot_key_long_press_event_source);
+
+        button_free(b2);
+        assert_se(!m->reboot_key_long_press_event_source);
+}
+
 int main(int argc, char **argv) {
         test_setup_logging(LOG_DEBUG);
 
@@ -33,6 +81,7 @@ int main(int argc, char **argv) {
         test_table(UserState, user_state, USER_STATE);
 
         test_sleep_handle_action();
+        test_button_free_cancels_long_press();
 
         return EXIT_SUCCESS;
 }