Re: [PATCH 4/4] HID: roccat-savu: reject short special reports
From: sashiko-bot@kernel.org
Date: 2026-06-28 16:49:40
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: Pre-existing issues: - [High] The driver acquires a mutex in the raw_event handler path, which executes in atomic (softIRQ) context, leading to a scheduling-while-atomic bug or deadlock. - [Medium] The roccat driver leaks the memory allocated for circular buffer reports when the device is destroyed. -- commit 60d1ec57aac96f7ad83f249887da4a3aa3a3f7c4 Author: Yousef Alhouseen [off-list ref] HID: roccat-savu: reject short special reports savu_report_to_chrdev() casts special reports to a five-byte structure and reads all of its payload fields without checking the received size. A malformed USB device can therefore trigger out-of-bounds reads from the input buffer when the character device is claimed. Pass the report size into the helper and require the complete structure.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/hid-roccat-savu.c b/drivers/hid/hid-roccat-savu.c index 6791369335601..04fa4c50cfa4b 100644 --- a/drivers/hid/hid-roccat-savu.c +++ b/drivers/hid/hid-roccat-savu.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -183,7 +184,7 @@ static int savu_raw_event(struct hid_device *hdev, return 0; if (savu->roccat_claimed) - savu_report_to_chrdev(savu, data); + savu_report_to_chrdev(savu, data, size);
[Severity: High]
This is a pre-existing issue, but can this lead to a scheduling-while-atomic
bug?
The savu_raw_event() callback runs in atomic context (softIRQ). When it
calls savu_report_to_chrdev(), the execution path proceeds to
roccat_report_event() in drivers/hid/hid-roccat.c, which acquires
sleeping locks:
drivers/hid/hid-roccat.c:roccat_report_event() {
...
mutex_lock(&device->readers_lock);
mutex_lock(&device->cbuf_lock);
...
}
[Severity: Medium]
This is also a pre-existing issue, but does this code leak the memory
allocated for circular buffer reports when the device is destroyed?
In drivers/hid/hid-roccat.c:roccat_report_event(), kmemdup() dynamically
allocates memory for the report:
new_value = kmemdup(data, device->report_size, GFP_ATOMIC);
However, when the device is freed in roccat_release() or
roccat_disconnect() using kfree(device), the dynamically allocated report
values in the circular buffer do not appear to be cleaned up.
return 0; }
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260628163527.14279-1-alhouseenyousef@gmail.com?part=4