* [PATCH v2 0/2] Input: discard pre-registration events and test state seeding @ 2026-10-10 19:10 Karlos Abel 2026-10-10 19:10 ` [PATCH v2 1/2] Input: discard pre-registration events before attaching handlers Karlos Abel 2026-10-10 19:10 ` [PATCH v2 2/2] Input: test state seeding across device registration Karlos Abel 0 siblings, 2 replies; 4+ messages in thread From: Karlos Abel @ 2026-10-10 19:10 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, sashiko-bot This series prevents input handlers from receiving buffered initialization events after device registration. On a ThinkPad T14 Gen 6, a later LED update flushed a stale SW_RFKILL_ALL event seeded by thinkpad_acpi. The fix drops the pending events and their timestamp before handlers attach, while retaining the seeded switch and absolute-axis state. Patch 1 contains the six-line runtime fix. Patch 2 adds KUnit coverage for registration with and without event-buffer resizing. The test uses a virtual SW_DOCK device and performs no real radio operations. Tested on mainline 3857c2fe5449541e24afc5efdb0f81a8a8f9a3a0 (7.3.0-rc6): - Tests alone on the unmodified input core: four pass, two fail. The non-resizing case replays one stale switch event; both cases retain the initialization timestamp. - Patch 1 alone: all four existing input-core tests pass. - Both patches: all six input-core tests pass. Both source commits were built and tested under UML without compiler warnings. The revised tests repeat the same failing/passing comparison. The same runtime fix was also tested with QEMU/KVM on Arch 7.2.7 and on the ThinkPad. AI assistance: OpenAI Codex assisted source analysis, the fix, regression tests and this text. The submitter performed the physical hardware comparisons. Previous version: Message-ID: <20261010180031.7-1-kabel@voidship.net> Review addressed: Message-ID: <sashiko-outbox-166792@kernel.org> Changes in v2: - Patch 1 is unchanged. - Patch 2 checks absinfo allocation before event seeding and unregisters its handler before its device, addressing Sashiko's review of v1. - Forced allocation failure now reaches the explicit check and cleanup. A forced teardown interleaving reproduced unintended binding after actual allocator address reuse; handler-first cleanup prevents it. Karlos Abel (2): Input: discard pre-registration events before attaching handlers Input: test state seeding across device registration drivers/input/input.c | 6 ++ drivers/input/tests/input_test.c | 171 +++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+) base-commit: 3857c2fe5449541e24afc5efdb0f81a8a8f9a3a0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/2] Input: discard pre-registration events before attaching handlers 2026-10-10 19:10 [PATCH v2 0/2] Input: discard pre-registration events and test state seeding Karlos Abel @ 2026-10-10 19:10 ` Karlos Abel 2026-10-10 19:17 ` sashiko-bot 2026-10-10 19:10 ` [PATCH v2 2/2] Input: test state seeding across device registration Karlos Abel 1 sibling, 1 reply; 4+ messages in thread From: Karlos Abel @ 2026-10-10 19:10 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, sashiko-bot input_event() permits drivers to seed switch and absolute-axis state before registration without delivering those events to input handlers. After event-buffer allocation moved to input_allocate_device(), such initialization events can remain queued across input_register_device() when the buffer does not need resizing. A later injected LED event followed by SYN_REPORT then delivers the old switch event to newly attached handlers. This was observed with the initial SW_RFKILL_ALL event from thinkpad_acpi on a ThinkPad T14 Gen 6: an LED update replayed the initial radio-switch event after registration. Registration can also leave a pre-registration timestamp attached to the first real event packet, including when the event buffer is resized. Clear the pending event count and monotonic timestamp under event_lock before attaching handlers. Keep the switch and absolute-axis state that was seeded by those events. Resetting the monotonic timestamp follows the existing flush path and lets the next packet obtain a fresh timestamp. Fixes: 0cd587735205 ("Input: preallocate memory to hold event values") Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Karlos Abel <kabel@voidship.net> --- drivers/input/input.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/input/input.c b/drivers/input/input.c index 01c91fec9..d59d62caa 100644 --- a/drivers/input/input.c +++ b/drivers/input/input.c @@ -2445,6 +2445,12 @@ int input_register_device(struct input_dev *dev) error = -EINTR; scoped_cond_guard(mutex_intr, goto err_device_del, &input_mutex) { + /* Keep seeded state, but do not replay pre-registration events. */ + scoped_guard(spinlock_irq, &dev->event_lock) { + dev->num_vals = 0; + dev->timestamp[INPUT_CLK_MONO] = ktime_set(0, 0); + } + list_add_tail(&dev->node, &input_dev_list); list_for_each_entry(handler, &input_handler_list, node) ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/2] Input: discard pre-registration events before attaching handlers 2026-10-10 19:10 ` [PATCH v2 1/2] Input: discard pre-registration events before attaching handlers Karlos Abel @ 2026-10-10 19:17 ` sashiko-bot 0 siblings, 0 replies; 4+ messages in thread From: sashiko-bot @ 2026-10-10 19:17 UTC (permalink / raw) To: Karlos Abel; +Cc: linux-input, dmitry.torokhov Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] input: mixed goto-based error cleanup and scope-based guards -- commit 4198890f3caadca9048a77f9b7d142bbae15ab35 Author: Karlos Abel <kabel@voidship.net> Input: discard pre-registration events before attaching handlers This commit prevents replaying pre-registration events when an input handler connects. It does this by clearing the pending event count and monotonic timestamp under event_lock before attaching handlers. > diff --git a/drivers/input/input.c b/drivers/input/input.c > index 01c91fec9b7eb..d59d62caa2f92 100644 > --- a/drivers/input/input.c > +++ b/drivers/input/input.c > @@ -2445,6 +2445,12 @@ int input_register_device(struct input_dev *dev) > > error = -EINTR; > scoped_cond_guard(mutex_intr, goto err_device_del, &input_mutex) { > + /* Keep seeded state, but do not replay pre-registration events. */ > + scoped_guard(spinlock_irq, &dev->event_lock) { > + dev->num_vals = 0; > + dev->timestamp[INPUT_CLK_MONO] = ktime_set(0, 0); > + } > + [Severity: Low] Does this mix scope-based cleanup with goto-based error handling? The kernel cleanup guidelines in include/linux/cleanup.h and the subsystem rules state that we should either convert all resources requiring a "goto" cleanup to scope-based cleanup, or convert none of them. Mixing both in the same function creates confusing ownership semantics and increases the risk of double-free or resource leak bugs. Because input_register_device() uses manual goto labels (err_device_del, err_devres_free) for its failure paths, does adding new scope-based cleanup macros like scoped_guard() here violate the cleanup guidelines? > list_add_tail(&dev->node, &input_dev_list); > > list_for_each_entry(handler, &input_handler_list, node) -- Sashiko AI review · https://sashiko.dev/#/patchset/20261010191033.23577-1-kabel@voidship.net?part=1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] Input: test state seeding across device registration 2026-10-10 19:10 [PATCH v2 0/2] Input: discard pre-registration events and test state seeding Karlos Abel 2026-10-10 19:10 ` [PATCH v2 1/2] Input: discard pre-registration events before attaching handlers Karlos Abel @ 2026-10-10 19:10 ` Karlos Abel 1 sibling, 0 replies; 4+ messages in thread From: Karlos Abel @ 2026-10-10 19:10 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, sashiko-bot Add KUnit coverage for input events used to initialize device state before registration. Register a handler for a virtual switch/LED device, seed a switch before input_register_device(), and inject a later LED update and SYN_REPORT. Verify that the initial state is retained without replaying its event or reusing its timestamp, and that a real later change arrives. Exercise both the original event buffer and registration with a larger buffer. Also verify preservation and subsequent delivery of absolute-axis state in the resizing case. Use SW_DOCK to avoid real radio operations. These tests depend on the preceding pre-registration event fix. Against the unpatched input core, the non-resizing case delivers one stale switch event and both cases retain the initialization timestamp. Both tests pass with the fix, together with the four existing input-core tests. Assisted-by: LLM Signed-off-by: Karlos Abel <kabel@voidship.net> --- Changes in v2: - Check absinfo allocation before seeding ABS_X and free the test device if allocation failed. - Unregister the test handler before unregistering the device, so its pointer matcher cannot bind to a replacement at the freed address. drivers/input/tests/input_test.c | 171 +++++++++++++++++++++++++++++++ 1 file changed, 171 insertions(+) diff --git a/drivers/input/tests/input_test.c b/drivers/input/tests/input_test.c index e105ce71a..fe4beec8e 100644 --- a/drivers/input/tests/input_test.c +++ b/drivers/input/tests/input_test.c @@ -161,11 +161,182 @@ static void input_test_grab(struct kunit *test) input_put_device(input_dev); } +struct input_seed_test { + struct input_dev *dev; + struct input_handler handler; + struct input_handle handle; + unsigned int switch_events; + unsigned int abs_events; + ktime_t led_timestamp; + bool connected; +}; + +static void input_seed_event(struct input_handle *handle, unsigned int type, + unsigned int code, int value) +{ + struct input_seed_test *seed = handle->private; + + if (type == EV_SW && code == SW_DOCK) + seed->switch_events++; + if (type == EV_ABS && code == ABS_X) + seed->abs_events++; + if (type == EV_LED && code == LED_NUML) + seed->led_timestamp = input_get_timestamp(handle->dev)[INPUT_CLK_MONO]; +} + +static bool input_seed_match(struct input_handler *handler, + struct input_dev *dev) +{ + struct input_seed_test *seed = container_of(handler, struct input_seed_test, + handler); + + return seed->dev == dev; +} + +static int input_seed_connect(struct input_handler *handler, + struct input_dev *dev, + const struct input_device_id *id) +{ + struct input_seed_test *seed = container_of(handler, struct input_seed_test, + handler); + int error; + + seed->handle.dev = dev; + seed->handle.handler = handler; + seed->handle.name = "input-seed-test"; + seed->handle.private = seed; + error = input_register_handle(&seed->handle); + if (error) + return error; + + error = input_open_device(&seed->handle); + if (error) { + input_unregister_handle(&seed->handle); + return error; + } + + seed->connected = true; + return 0; +} + +static void input_seed_disconnect(struct input_handle *handle) +{ + input_close_device(handle); + input_unregister_handle(handle); +} + +static const struct input_device_id input_seed_ids[] = { + { .flags = INPUT_DEVICE_ID_MATCH_BUS, .bustype = BUS_VIRTUAL }, + { } +}; + +static void input_test_seed_events(struct kunit *test, bool resize) +{ + const ktime_t seed_timestamp = ktime_set(123456, 789); + struct input_seed_test *seed; + int error; + + seed = kunit_kzalloc(test, sizeof(*seed), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, seed); + seed->dev = input_allocate_device(); + KUNIT_ASSERT_NOT_NULL(test, seed->dev); + + seed->dev->name = "Input pre-registration events"; + seed->dev->id.bustype = BUS_VIRTUAL; + input_set_capability(seed->dev, EV_SW, SW_DOCK); + input_set_capability(seed->dev, EV_LED, LED_NUML); + /* An ABS axis also raises the estimated event-buffer size. */ + if (resize) { + input_set_abs_params(seed->dev, ABS_X, 0, 100, 0, 0); + KUNIT_EXPECT_NOT_NULL(test, seed->dev->absinfo); + if (!seed->dev->absinfo) + goto free_device; + seed->dev->hint_events_per_packet = 128; + } + + seed->handler.event = input_seed_event; + seed->handler.match = input_seed_match; + seed->handler.connect = input_seed_connect; + seed->handler.disconnect = input_seed_disconnect; + seed->handler.name = "input-seed-test"; + seed->handler.id_table = input_seed_ids; + error = input_register_handler(&seed->handler); + KUNIT_EXPECT_EQ(test, error, 0); + if (error) + goto free_device; + + /* Initialize state before registration, without a SYN_REPORT. */ + input_set_timestamp(seed->dev, seed_timestamp); + input_report_switch(seed->dev, SW_DOCK, 1); + if (resize) + input_report_abs(seed->dev, ABS_X, 42); + error = input_register_device(seed->dev); + KUNIT_EXPECT_EQ(test, error, 0); + if (error) + goto unregister_handler; + + KUNIT_EXPECT_TRUE(test, seed->connected); + if (!seed->connected) + goto unregister_device; + + KUNIT_EXPECT_TRUE(test, test_bit(SW_DOCK, seed->dev->sw)); + if (resize) + KUNIT_EXPECT_EQ(test, input_abs_get_val(seed->dev, ABS_X), 42); + KUNIT_EXPECT_EQ(test, seed->switch_events, 0); + KUNIT_EXPECT_EQ(test, seed->abs_events, 0); + + /* A later LED write must not replay the initialization events. */ + input_inject_event(&seed->handle, EV_LED, LED_NUML, 1); + input_inject_event(&seed->handle, EV_SYN, SYN_REPORT, 0); + KUNIT_EXPECT_EQ(test, seed->switch_events, 0); + KUNIT_EXPECT_EQ(test, seed->abs_events, 0); + KUNIT_EXPECT_NE(test, seed->led_timestamp, seed_timestamp); + KUNIT_EXPECT_NE(test, seed->led_timestamp, ktime_set(0, 0)); + + /* Genuine post-registration state changes must still be delivered. */ + seed->switch_events = 0; + seed->abs_events = 0; + input_report_switch(seed->dev, SW_DOCK, 0); + if (resize) + input_report_abs(seed->dev, ABS_X, 77); + input_sync(seed->dev); + KUNIT_EXPECT_EQ(test, seed->switch_events, 1); + KUNIT_EXPECT_EQ(test, seed->abs_events, resize ? 1 : 0); + KUNIT_EXPECT_FALSE(test, test_bit(SW_DOCK, seed->dev->sw)); + if (resize) + KUNIT_EXPECT_EQ(test, input_abs_get_val(seed->dev, ABS_X), 77); + input_inject_event(&seed->handle, EV_SYN, SYN_REPORT, 0); + KUNIT_EXPECT_EQ(test, seed->switch_events, 1); + KUNIT_EXPECT_EQ(test, seed->abs_events, resize ? 1 : 0); + +unregister_device: + input_unregister_handler(&seed->handler); + input_unregister_device(seed->dev); + return; + +unregister_handler: + input_unregister_handler(&seed->handler); +free_device: + input_free_device(seed->dev); +} + +static void input_test_seed(struct kunit *test) +{ + input_test_seed_events(test, false); +} + +static void input_test_seed_resize(struct kunit *test) +{ + input_test_seed_events(test, true); +} + static struct kunit_case input_tests[] = { KUNIT_CASE(input_test_polling), KUNIT_CASE(input_test_timestamp), KUNIT_CASE(input_test_match_device_id), KUNIT_CASE(input_test_grab), + KUNIT_CASE(input_test_seed), + KUNIT_CASE(input_test_seed_resize), { /* sentinel */ } }; -- 2.56.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-10 19:17 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-10 19:10 [PATCH v2 0/2] Input: discard pre-registration events and test state seeding Karlos Abel 2026-10-10 19:10 ` [PATCH v2 1/2] Input: discard pre-registration events before attaching handlers Karlos Abel 2026-10-10 19:17 ` sashiko-bot 2026-10-10 19:10 ` [PATCH v2 2/2] Input: test state seeding across device registration Karlos Abel
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.