From 66816ac78a8600de6b3fae479e43e6bfc59313a8 Mon Sep 17 00:00:00 2001 From: mescon <5875228+mescon@users.noreply.github.com> Date: Sun, 4 Oct 2026 01:05:50 +0200 Subject: [PATCH] hidpp: match answers only against a live reply buffer The raw-event path took a held send_mutex to mean a sync sender was waiting, and matched incoming HID++ reports against send_receive_buf, the previous sender's response on that sender's stack. Four senders here hold the mutex with no response buffer (the OLED frame worker and handback, the rev-light level sender, the G923 rev-light worker, which sleeps under it), so during their hold the stale pointer was followed; once the owning process had exited and its stack was unmapped, the read faulted in interrupt context and the kernel panicked with nothing in the journal. Photographed on a G923 Xbox in AC EVO (#128), the freeze #90 chased: RIP hidpp_match_answer+0x8, RSI a vmap-stack address, CR2 that address plus one (question->device_index). Clear the pointer under the mutex before releasing it, and gate the match on a live pointer as well as the mutex. --- CHANGELOG.md | 16 ++++++++++++++++ mainline/hid-logitech-hidpp.c | 24 ++++++++++++++++++------ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7b7944..0cc104a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,22 @@ the contract is "it works on RS50 and G Pro as listed here". ## Unreleased +**A kernel panic on the G923 Xbox edition during long AC and AC EVO +sessions, fixed.** The driver's answer matcher took a held send mutex to +mean a command was waiting for its reply, and matched every incoming HID++ +report against the last reply buffer, which lives on the stack of whoever +sent the last command. Four senders in this driver (the OLED frame worker +and its handback, the rev-light level sender and the G923 rev-light +worker, which sleeps while it holds the mutex) hold that mutex without a +reply buffer of their own, so a report arriving during their hold was +compared against a stack frame that was long gone; once the process that +owned it had exited (session restarts spawn short-lived helpers that write +sysfs) the read faulted inside the USB interrupt and the kernel panicked, +leaving nothing in the journal. Caught on screen by a reporter after weeks +of hunting ([#128](../../issues/128), [#90](../../issues/90)). The reply +pointer is now cleared before the mutex is released, and the matcher only +runs with a live one. + **DiRT Rally 2.0's setup text carries the `device_defines.xml` line itself.** It pointed to the wiki for the exact line, and that wiki page had been emptied by a bad edit (restored), so an RS50 owner used the wheel's diff --git a/mainline/hid-logitech-hidpp.c b/mainline/hid-logitech-hidpp.c index fdd7bc7..0b5c8d9 100644 --- a/mainline/hid-logitech-hidpp.c +++ b/mainline/hid-logitech-hidpp.c @@ -705,7 +705,7 @@ static int __do_hidpp_send_message_sync(struct hidpp_device *hidpp, __must_hold(&hidpp->send_mutex); - hidpp->send_receive_buf = response; + WRITE_ONCE(hidpp->send_receive_buf, response); hidpp->answer_available = false; /* @@ -879,6 +879,16 @@ static int hidpp_send_message_sync_timeout(struct hidpp_device *hidpp, } while (--max_retries); hidpp_note_sync_result(hidpp, ret); + /* + * The response lives on this caller's stack. Drop the pointer + * before the mutex goes, or the next holder that is not a sync + * sender (the OLED and rev-light workers lock it to serialise + * their writes) leaves the raw-event path matching incoming + * reports against a frame that is gone, and once that stack is + * unmapped the match faults in interrupt context and the kernel + * panics (#128, the freeze #90 chased). + */ + WRITE_ONCE(hidpp->send_receive_buf, NULL); mutex_unlock(&hidpp->send_mutex); return ret; @@ -18487,17 +18497,19 @@ static int hidpp_input_configured(struct hid_device *hdev, static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data, int size) { - struct hidpp_report *question = hidpp->send_receive_buf; - struct hidpp_report *answer = hidpp->send_receive_buf; + struct hidpp_report *question = READ_ONCE(hidpp->send_receive_buf); + struct hidpp_report *answer = question; struct hidpp_report *report = (struct hidpp_report *)data; int ret; int last_online; /* - * If the mutex is locked then we have a pending answer from a - * previously sent command. + * A pending answer needs a sync sender waiting on it, and the + * mutex alone does not say there is one: the OLED and rev-light + * workers hold it too, with no buffer to answer into. Only a + * live buffer is a question (#128). */ - if (unlikely(mutex_is_locked(&hidpp->send_mutex))) { + if (unlikely(question && mutex_is_locked(&hidpp->send_mutex))) { /* * Check for a correct hidpp20 answer or the corresponding * error