* Re: [PATCH v2] HID: letsketch: fix UAF on inrange_timer at driver unbind
From: Jiri Kosina @ 2026-06-29 8:43 UTC (permalink / raw)
To: Manish Khadka
Cc: linux-input, Hans de Goede, Benjamin Tissoires, linux-kernel
In-Reply-To: <20260515164200.77039-1-maskmemanish@gmail.com>
On Fri, 15 May 2026, Manish Khadka wrote:
> letsketch_driver does not provide a .remove callback, but
> letsketch_probe() arms a per-device timer:
>
> timer_setup(&data->inrange_timer, letsketch_inrange_timeout, 0);
>
> The timer is re-armed from letsketch_raw_event() with a 100 ms
> timeout on every pen-in-range report, and its callback dereferences
> data->input_tablet to deliver a synthetic BTN_TOOL_PEN release.
>
> letsketch_data is allocated with devm_kzalloc(), and its input_dev
> fields are devm-allocated via letsketch_setup_input_tablet(). On
> device unbind (USB unplug or rmmod), the HID core runs its default
> teardown and devm cleanup frees both letsketch_data and the input
> devices. Because no .remove callback exists, nothing drains the
> timer first: if raw_event armed it within ~100 ms of the unbind,
> the pending timer fires on freed memory. This is a UAF read of
> data and of data->input_tablet, followed by input_report_key() /
> input_sync() into the freed input_dev.
>
> The same problem can occur on the probe error path: if
> hid_hw_start() enabled I/O on an always-poll-quirk device and then
> failed, raw_event may have armed the timer before devm releases
> data.
>
> Fix by adding a .remove callback that calls hid_hw_stop() first.
> hid_hw_stop() synchronously kills the URBs that deliver raw_event(),
> so once it returns no path can re-arm the timer. timer_shutdown_sync()
> then drains any in-flight callback and permanently disables further
> mod_timer() calls. Apply the same timer_shutdown_sync() in the probe
> error path so the timer is guaranteed not to outlive data.
>
> Fixes: 33a5c2793451 ("HID: Add new Letsketch tablet driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Manish Khadka <maskmemanish@gmail.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] Input: maplemouse - fix NULL pointer dereference in open()
From: sashiko-bot @ 2026-06-28 23:27 UTC (permalink / raw)
To: Florian Fuchs; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628230715.2982552-1-fuchsfl@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Initialization of maple bus driver data occurs after exposing the input device to userspace, leading to a potential NULL pointer dereference.
- [High] Teardown asymmetry causes a race condition where a background callback is inadvertently re-armed during device removal, leading to UAF or NULL pointer dereference.
--
commit 5babe5754528798c9e3476330c1a7834fe83388a
Author: Florian Fuchs <fuchsfl@gmail.com>
Input: maplemouse - fix NULL pointer dereference in open()
Commit 555c765b0cc2 dropped the input_set_drvdata() call in probe, which
caused a NULL pointer dereference in open() because maple_get_drvdata()
was still being used. This patch restores input_set_drvdata() and
converts open() and close() to use input_get_drvdata().
> diff --git a/drivers/input/mouse/maplemouse.c b/drivers/input/mouse/maplemouse.c
> index c99f7e2342191..c41182766538a 100644
> --- a/drivers/input/mouse/maplemouse.c
> +++ b/drivers/input/mouse/maplemouse.c
[ ... ]
> @@ -58,7 +58,7 @@ static int dc_mouse_open(struct input_dev *dev)
>
> static void dc_mouse_close(struct input_dev *dev)
> {
> - struct dc_mouse *mse = maple_get_drvdata(to_maple_dev(&dev->dev));
> + struct dc_mouse *mse = input_get_drvdata(dev);
>
> maple_getcond_callback(mse->mdev, dc_mouse_callback, 0,
> MAPLE_FUNC_MOUSE);
> }
[Severity: High]
This is a pre-existing issue, but does calling maple_getcond_callback() here
create a race condition during device removal?
During removal, remove_maple_mouse() clears the callback before unregistering
the device:
drivers/input/mouse/maplemouse.c:remove_maple_mouse() {
...
mdev->callback = NULL;
input_unregister_device(mse->dev);
...
}
If the device is open, input_unregister_device() synchronously calls
dc_mouse_close(). This unconditionally calls maple_getcond_callback(), which
re-arms mdev->callback back to dc_mouse_callback.
After input_unregister_device() returns, remove_maple_mouse() clears the
driver data and frees mse. If a delayed maple bus packet arrives after this,
dc_mouse_callback() will execute and attempt to use a freed mse pointer or a
NULL driver data pointer, causing a kernel panic.
Would it be safer to avoid re-arming the callback if the device is being
removed?
[ ... ]
> @@ -88,6 +88,7 @@ static int probe_maple_mouse(struct device *dev)
> mse->dev = input_dev;
> mse->mdev = mdev;
>
> + input_set_drvdata(input_dev, mse);
> input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
> input_dev->keybit[BIT_WORD(BTN_MOUSE)] = BIT_MASK(BTN_LEFT) |
> BIT_MASK(BTN_RIGHT) | BIT_MASK(BTN_MIDDLE);
[Severity: High]
This is a pre-existing issue, but is it possible for a callback to fire
before driver data is fully initialized?
Later in probe_maple_mouse(), the device is registered:
drivers/input/mouse/maplemouse.c:probe_maple_mouse() {
...
error = input_register_device(input_dev);
if (error)
goto fail_register;
mdev->driver = mdrv;
maple_set_drvdata(mdev, mse);
...
}
If userspace immediately opens the device upon registration, dc_mouse_open()
will schedule dc_mouse_callback(). If the callback executes before
maple_set_drvdata() completes, maple_get_drvdata(mapledev) inside the
callback will return NULL, leading to a NULL pointer dereference when
accessing mse->dev.
Could the maple_set_drvdata() call be moved before input_register_device()
to prevent this?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628230715.2982552-1-fuchsfl@gmail.com?part=1
^ permalink raw reply
* [PATCH] Input: maplemouse - fix NULL pointer dereference in open()
From: Florian Fuchs @ 2026-06-28 23:07 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: linux-sh, Guenter Roeck, linux-kernel, Florian Fuchs
Commit 555c765b0cc2 ("Input: mouse - drop unnecessary calls to
input_set_drvdata") dropped the input_set_drvdata() call in probe
because the data appeared to be unused. However, dc_mouse_open() and
dc_mouse_close() were using maple_get_drvdata(to_maple_dev(&dev->dev)).
This actually retrieves driver data from the input device's embedded
struct device. After input_set_drvdata() was removed, that lookup started
returning NULL and opening the input device dereferences mse->mdev.
Restore input_set_drvdata() and convert open() and close() to use
input_get_drvdata() so the dependency is no longer hidden.
Fixes: 555c765b0cc2 ("Input: mouse - drop unnecessary calls to input_set_drvdata")
Signed-off-by: Florian Fuchs <fuchsfl@gmail.com>
---
This fix was tested on the target platform. The following is the error I
get, when using the unpatched kernel:
BUG: unable to handle kernel NULL pointer dereference at 00000004
PC: [<8c26eec4>] dc_mouse_open+0xc/0x28
pgd = f700ee57
[00000004] *pgd=00000000
Oops: 0000 [#1]
CPU: 0 UID: 0 PID: 45 Comm: Xfbdev Not tainted 7.1.1 #84 PREEMPT
PC is at dc_mouse_open+0xc/0x28
PR is at input_open_device+0x7c/0xe0
PC : 8c26eec4 SP : 8c7bbd9c SR : 40008100 TEA : 00000004
R0 : 8c26eeb8 R1 : 00000000 R2 : 00000001 R3 : 00000000
R4 : 8c6b0dc0 R5 : 8c26efa8 R6 : 8c7b64c0 R7 : 00000200
R8 : 00000000 R9 : 8c6b0d70 R10 : 8c6b0c00 R11 : 8c6ce604
R12 : 8c390a64 R13 : 8c6b0d3c R14 : 8c0e9ba0
MACH: 00000006 MACL: 8686868d GBR : 29609ff4 PR : 8c265fc8
Call trace:
[<8c265fc8>] input_open_device+0x7c/0xe0
[<8c26b2d0>] mousedev_open_device+0x38/0x68
[<8c26b77c>] mousedev_open+0xa4/0x110
[<8c0e9cc6>] chrdev_open+0x112/0x15c
[<8c0e2e42>] do_dentry_open+0x27e/0x2fc
[<8c0e9bb4>] chrdev_open+0x0/0x15c
[<8c0f32d2>] path_openat+0x1d2/0x7cc
[<8c0f3956>] do_file_open+0x8a/0xf0
[<8c0f3100>] path_openat+0x0/0x7cc
[<8c1efeac>] strncpy_from_user+0x64/0xe4
[<8c0ffc7e>] alloc_fd+0x106/0x124
[<8c0e41ed>] sys_openat2+0xb9/0xbc
[<8c0e3fc6>] do_sys_openat2+0x76/0xd4
[<8c0e40ee>] do_sys_open+0x2a/0x54
[<8c00e25a>] syscall_call+0x18/0x1e
[<8c0e4118>] sys_open+0x0/0x10
drivers/input/mouse/maplemouse.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/input/mouse/maplemouse.c b/drivers/input/mouse/maplemouse.c
index c99f7e234219..c41182766538 100644
--- a/drivers/input/mouse/maplemouse.c
+++ b/drivers/input/mouse/maplemouse.c
@@ -48,7 +48,7 @@ static void dc_mouse_callback(struct mapleq *mq)
static int dc_mouse_open(struct input_dev *dev)
{
- struct dc_mouse *mse = maple_get_drvdata(to_maple_dev(&dev->dev));
+ struct dc_mouse *mse = input_get_drvdata(dev);
maple_getcond_callback(mse->mdev, dc_mouse_callback, HZ/50,
MAPLE_FUNC_MOUSE);
@@ -58,7 +58,7 @@ static int dc_mouse_open(struct input_dev *dev)
static void dc_mouse_close(struct input_dev *dev)
{
- struct dc_mouse *mse = maple_get_drvdata(to_maple_dev(&dev->dev));
+ struct dc_mouse *mse = input_get_drvdata(dev);
maple_getcond_callback(mse->mdev, dc_mouse_callback, 0,
MAPLE_FUNC_MOUSE);
@@ -88,6 +88,7 @@ static int probe_maple_mouse(struct device *dev)
mse->dev = input_dev;
mse->mdev = mdev;
+ input_set_drvdata(input_dev, mse);
input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REL);
input_dev->keybit[BIT_WORD(BTN_MOUSE)] = BIT_MASK(BTN_LEFT) |
BIT_MASK(BTN_RIGHT) | BIT_MASK(BTN_MIDDLE);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] HID: hiddev: keep state alive through disconnect unlock
From: Hillf Danton @ 2026-06-28 22:26 UTC (permalink / raw)
To: Yousef Alhouseen
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-usb,
linux-kernel, syzbot+563191a4939ddbfe73d4
In-Reply-To: <20260628093245.42065-1-alhouseenyousef@gmail.com>
On Sun, 28 Jun 2026 11:32:45 +0200 Yousef Alhouseen wrote:
> hiddev_disconnect() drops existancelock before freeing the hiddev state,
> but a waiting final file release can run as soon as the mutex becomes
> available. On PREEMPT_RT, that waiter may free hiddev while the disconnect
> thread is still executing the mutex slow-unlock path, causing a
> use-after-free in the mutex implementation.
>
The root cause is not specified as clearly as expected.
hiddev_disconnect() hiddev_release()
--- ---
mutex_lock(&hiddev->existancelock);
hiddev->exist = 0;
if (hiddev->open) {
hid_hw_close(hiddev->hid);
wake_up_interruptible(&hiddev->wait);
mutex_unlock(&hiddev->existancelock);
__mutex_unlock_slowpath()
raw_spin_lock_irqsave() //UAF
mutex_lock(&list->hiddev->existancelock);
if (!--list->hiddev->open) {
if (list->hiddev->exist) {
hid_hw_close(list->hiddev->hid);
hid_hw_power(list->hiddev->hid, PM_HINT_NORMAL);
} else {
mutex_unlock(&list->hiddev->existancelock);
kfree(list->hiddev); // free mem
vfree(list);
return 0;
}
}
mutex_unlock(&list->hiddev->existancelock);
Given the syzbot report, uaf occured upon acquiring the raw spinlock in
__mutex_unlock_slowpath(), but no mutex waiter could be waken up without
the raw spinlock held, thus the report sounds false positive.
> Give the connection and each open file an explicit reference. Drop each
> reference only after its existancelock critical section has completed, so
> the state cannot be freed from the other unlock path.
>
> Fixes: 079034073faf ("HID: hiddev cleanup -- handle all error conditions properly")
> Reported-by: syzbot+563191a4939ddbfe73d4@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=563191a4939ddbfe73d4
> Cc: stable@vger.kernel.org
> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
> ---
> drivers/hid/usbhid/hiddev.c | 37 +++++++++++++++++++------------------
> include/linux/hiddev.h | 2 ++
> 2 files changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/hid/usbhid/hiddev.c b/drivers/hid/usbhid/hiddev.c
> index 6378801b22c6..21396481995b 100644
> --- a/drivers/hid/usbhid/hiddev.c
> +++ b/drivers/hid/usbhid/hiddev.c
> @@ -46,6 +46,12 @@ struct hiddev_list {
> struct mutex thread_lock;
> };
>
> +static void hiddev_put(struct hiddev *hiddev)
> +{
> + if (refcount_dec_and_test(&hiddev->refcount))
> + kfree(hiddev);
> +}
> +
> /*
> * Find a report, given the report's type and ID. The ID can be specified
> * indirectly by REPORT_ID_FIRST (which returns the first report of the given
> @@ -216,26 +222,21 @@ static int hiddev_fasync(int fd, struct file *file, int on)
> static int hiddev_release(struct inode * inode, struct file * file)
> {
> struct hiddev_list *list = file->private_data;
> + struct hiddev *hiddev = list->hiddev;
> unsigned long flags;
>
> - spin_lock_irqsave(&list->hiddev->list_lock, flags);
> + spin_lock_irqsave(&hiddev->list_lock, flags);
> list_del(&list->node);
> - spin_unlock_irqrestore(&list->hiddev->list_lock, flags);
> + spin_unlock_irqrestore(&hiddev->list_lock, flags);
>
> - mutex_lock(&list->hiddev->existancelock);
> - if (!--list->hiddev->open) {
> - if (list->hiddev->exist) {
> - hid_hw_close(list->hiddev->hid);
> - hid_hw_power(list->hiddev->hid, PM_HINT_NORMAL);
> - } else {
> - mutex_unlock(&list->hiddev->existancelock);
> - kfree(list->hiddev);
> - vfree(list);
> - return 0;
> - }
> + mutex_lock(&hiddev->existancelock);
> + if (!--hiddev->open && hiddev->exist) {
> + hid_hw_close(hiddev->hid);
> + hid_hw_power(hiddev->hid, PM_HINT_NORMAL);
> }
>
> - mutex_unlock(&list->hiddev->existancelock);
> + mutex_unlock(&hiddev->existancelock);
> + hiddev_put(hiddev);
> vfree(list);
>
> return 0;
> @@ -270,6 +271,7 @@ static int __hiddev_open(struct hiddev *hiddev, struct file *file)
> spin_unlock_irq(&hiddev->list_lock);
>
> file->private_data = list;
> + refcount_inc(&hiddev->refcount);
>
> return 0;
>
> @@ -897,6 +899,7 @@ int hiddev_connect(struct hid_device *hid, unsigned int force)
> INIT_LIST_HEAD(&hiddev->list);
> spin_lock_init(&hiddev->list_lock);
> mutex_init(&hiddev->existancelock);
> + refcount_set(&hiddev->refcount, 1);
> hid->hiddev = hiddev;
> hiddev->hid = hid;
> hiddev->exist = 1;
> @@ -937,9 +940,7 @@ void hiddev_disconnect(struct hid_device *hid)
> if (hiddev->open) {
> hid_hw_close(hiddev->hid);
> wake_up_interruptible(&hiddev->wait);
> - mutex_unlock(&hiddev->existancelock);
> - } else {
> - mutex_unlock(&hiddev->existancelock);
> - kfree(hiddev);
> }
> + mutex_unlock(&hiddev->existancelock);
> + hiddev_put(hiddev);
> }
> diff --git a/include/linux/hiddev.h b/include/linux/hiddev.h
> index 2164c03d2c72..8e9f8a33e359 100644
> --- a/include/linux/hiddev.h
> +++ b/include/linux/hiddev.h
> @@ -13,6 +13,7 @@
> #ifndef _HIDDEV_H
> #define _HIDDEV_H
>
> +#include <linux/refcount.h>
> #include <uapi/linux/hiddev.h>
>
>
> @@ -24,6 +25,7 @@ struct hiddev {
> int minor;
> int exist;
> int open;
> + refcount_t refcount;
> struct mutex existancelock;
> wait_queue_head_t wait;
> struct hid_device *hid;
> --
> 2.54.0
^ permalink raw reply
* Re: [PATCH 6/6] HID: roccat-ryos: reject short special reports
From: sashiko-bot @ 2026-06-28 17:01 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260628164611.17467-6-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The `roccat_report_event()` function acquires sleepable mutexes while running in an atomic context (USB URB completion handler), which leads to a "scheduling while atomic" BUG.
--
commit de88f5cf3ef84f642bf5e1815c5ed0d7ee05a061
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: roccat-ryos: reject short special reports
This commit fixes an out-of-bounds read in ryos_raw_event() by ensuring
that only complete special reports are forwarded to the character-device
layer. It drops reports that are shorter than the expected report size.
> diff --git a/drivers/hid/hid-roccat-ryos.c b/drivers/hid/hid-roccat-ryos.c
> index db83f42457da5..5087defc7c373 100644
> --- a/drivers/hid/hid-roccat-ryos.c
> +++ b/drivers/hid/hid-roccat-ryos.c
> @@ -189,7 +189,8 @@ static int ryos_raw_event(struct hid_device *hdev,
> != RYOS_USB_INTERFACE_PROTOCOL)
> return 0;
>
> - if (data[0] != RYOS_REPORT_NUMBER_SPECIAL)
> + if (data[0] != RYOS_REPORT_NUMBER_SPECIAL ||
> + size < sizeof(struct ryos_report_special))
> return 0;
>
> if (ryos != NULL && ryos->roccat_claimed)
> roccat_report_event(ryos->chrdev_minor, data);
[Severity: High]
This is a pre-existing issue, but is it safe to call roccat_report_event()
from this context?
The ryos_raw_event() callback executes in an atomic context (softirq)
during the USB URB completion handler:
hid_irq_in()
hid_safe_input_report()
__hid_input_report()
hdrv->raw_event() -> ryos_raw_event()
However, looking at drivers/hid/hid-roccat.c, roccat_report_event() acquires
sleepable mutexes:
roccat_report_event()
{
...
mutex_lock(&device->readers_lock);
mutex_lock(&device->cbuf_lock);
...
}
Will this sequence trigger a scheduling while atomic bug when a valid
special report is received and forwarded?
>
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628164611.17467-1-alhouseenyousef@gmail.com?part=6
^ permalink raw reply
* Re: [PATCH 1/6] HID: roccat-koneplus: reject short button reports
From: sashiko-bot @ 2026-06-28 17:01 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260628164611.17467-1-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Use-After-Free during device teardown due to incorrect removal ordering.
- [High] Incomplete Fix: Out-of-bounds read via short USB reports in the savu driver.
- [High] Memory leak of `struct roccat_reader` and `struct roccat_device` on hot-unplug.
- [High] Sleeping in atomic context due to mutex_lock in the raw_event path.
- [High] Lockless Access and Data Race on the `devices` array causing NULL pointer dereference or UAF.
- [High] Kernel information leak via sysfs attributes due to uninitialized memory copying and ignored errors.
- [Low] Unsynchronized access to `actual_profile` between process and interrupt context.
--
commit d5a3862120a746721ced4fff1b161538d9d1d57c
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: roccat-koneplus: reject short button reports
The Koneplus raw-event path casts button reports to an eight-byte
structure before updating profile state and forwarding an event. A
malformed USB device can identify a shorter report as a button report
and trigger out-of-bounds reads.
Require the complete button report before either consumer sees it.
Fixes: 47dbdbffe15b ("HID: roccat: Add support for Roccat Kone[+] v2")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
> diff --git a/drivers/hid/hid-roccat-koneplus.c b/drivers/hid/hid-roccat-koneplus.c
> index f80a60539a967..e0f35251e81a9 100644
> --- a/drivers/hid/hid-roccat-koneplus.c
> +++ b/drivers/hid/hid-roccat-koneplus.c
> @@ -523,6 +523,10 @@ static int koneplus_raw_event(struct hid_device *hdev,
> if (koneplus == NULL)
> return 0;
>
> + if (data[0] == KONEPLUS_MOUSE_REPORT_NUMBER_BUTTON &&
> + size < sizeof(struct koneplus_mouse_report_button))
> + return 0;
> +
[Severity: High]
This is a pre-existing issue, but does the savu driver also need a similar
check?
It looks like the savu driver lacks length validation. savu_raw_event()
forwards reports to savu_report_to_chrdev(), which casts the buffer:
drivers/hid/hid-roccat-savu.c:savu_report_to_chrdev() {
...
roccat_report.type = special_report->type;
roccat_report.data[0] = special_report->data[0];
roccat_report.data[1] = special_report->data[1];
...
}
If a malicious USB device sends a 1-byte report to the savu driver, could
these accesses read out-of-bounds of the heap buffer and leak information
to userspace via the character device?
> koneplus_keep_values_up_to_date(koneplus, data);
[Severity: Low]
This isn't a bug introduced by this patch, but is it safe to update the
profile state here without synchronization?
koneplus->actual_profile is updated from sysfs in process context under
koneplus_lock, but it is also updated here in interrupt context via
koneplus_keep_values_up_to_date() without acquiring any locks:
drivers/hid/hid-roccat-koneplus.c:koneplus_keep_values_up_to_date() {
...
koneplus_profile_activated(koneplus, button_report->data1 - 1);
...
}
Could this data race cause stale profile values to be read if sysfs
attributes are accessed concurrently with profile-changing USB events?
>
> if (koneplus->roccat_claimed)
[Severity: High]
This is a pre-existing issue, but can this path sleep in atomic context?
koneplus_raw_event() runs in atomic interrupt context from the USB URB
completion handler. If roccat_claimed is true, it calls
koneplus_report_to_chrdev(), which eventually calls roccat_report_event():
drivers/hid/hid-roccat.c:roccat_report_event() {
...
mutex_lock(&device->readers_lock);
mutex_lock(&device->cbuf_lock);
...
}
Would acquiring these mutexes inside the raw_event path trigger a
scheduling-while-atomic bug upon receiving valid input events?
[Severity: High]
This is another pre-existing issue, but does roccat_report_event() have a
data race with device removal?
drivers/hid/hid-roccat.c:roccat_report_event() {
...
device = devices[minor];
new_value = kmemdup(data, device->report_size, GFP_ATOMIC);
...
}
If roccat_disconnect() runs concurrently, takes devices_lock, and sets
devices[minor] = NULL, could roccat_report_event() fetch a NULL or freed
device pointer locklessly, leading to a crash or use-after-free when
accessing device->report_size?
I also noticed a few other pre-existing issues in the ROCCAT subsystem
that might be worth addressing:
[Severity: Critical]
This is a pre-existing issue, but could there be a use-after-free during
device teardown?
In koneplus_remove(), the driver state is freed before the hardware is
stopped:
drivers/hid/hid-roccat-koneplus.c:koneplus_remove() {
...
koneplus_remove_specials(hdev);
hid_hw_stop(hdev);
...
}
If a USB event arrives exactly during this window before hid_hw_stop()
halts the in-flight USB URBs, koneplus_raw_event() will fetch the
freed driver data:
drivers/hid/hid-roccat-koneplus.c:koneplus_raw_event() {
struct koneplus_device *koneplus = hid_get_drvdata(hdev);
...
koneplus_keep_values_up_to_date(koneplus, data);
...
}
Would this dereference the freed koneplus pointer?
[Severity: High]
This is a pre-existing issue, but does hot-unplugging the device while it
is held open by userspace cause a memory leak?
When a ROCCAT device is unplugged, roccat_disconnect() clears the array slot:
drivers/hid/hid-roccat.c:roccat_disconnect() {
...
devices[minor] = NULL;
...
}
If a user-space process holds the char device open, the memory isn't freed
immediately. When the process later closes the file descriptor,
roccat_release() checks the array:
drivers/hid/hid-roccat.c:roccat_release() {
...
device = devices[minor];
if (!device) {
mutex_unlock(&devices_lock);
pr_emerg("roccat device with minor %d doesn't exist\n", minor);
return -ENODEV;
}
...
}
Since the device was set to NULL during disconnect, it returns early and
bypasses freeing the reader and device structures. Are these structures
leaked permanently?
[Severity: High]
This is a pre-existing issue, but can sysfs attributes leak uninitialized
heap memory to userspace?
In roccat_common2_receive(), a buffer is allocated via kmalloc() without
GFP_ZERO. If usb_control_msg() fails, it still executes memcpy():
drivers/hid/hid-roccat-common.c:roccat_common2_receive() {
...
len = usb_control_msg(...);
memcpy(data, buf, size);
...
}
Sysfs show handlers ignore the return value and emit the potentially
uninitialized data:
drivers/hid/hid-roccat-koneplus.c:koneplus_sysfs_show_firmware_version() {
...
roccat_common2_receive(usb_dev, KONEPLUS_COMMAND_INFO,
&info, KONEPLUS_SIZE_INFO);
...
return sysfs_emit(buf, "%d\n", info.firmware_version);
}
If the USB transfer fails, does this expose uninitialized memory to any
local user able to read the sysfs files?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628164611.17467-1-alhouseenyousef@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH] HID: cp2112: validate input response lengths
From: sashiko-bot @ 2026-06-28 17:00 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628164731.17614-1-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] The irqchip callback `irq_shutdown` causes a system panic by sleeping in atomic context.
- [High] Concurrent masking and unmasking corrupts `dev->irq_mask`.
- [High] Stale state reads occur due to missing memory ordering between data updates and the availability flag.
- [High] Late USB responses from aborted I2C transfers wrongly satisfy new transfer wait conditions.
--
commit 4d019b9aa500a3c35cd141dda658a1f88ad1c46a
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: cp2112: validate input response lengths
cp2112_raw_event() parses transfer-status responses as a fixed seven-byte
structure without checking the report size. It also trusts the length in
data-read responses and copies that many bytes even when the received
report is shorter. A malformed USB device can use either path to trigger
out-of-bounds reads from the HID input buffer.
Reject short status responses with -EMSGSIZE. Treat truncated data
responses as zero-length reads so the waiting transfer fails instead of
timing out or copying beyond the report.
Fixes: e932d8178667 ("HID: add hid-cp2112 driver")
Cc: stable@vger.kernel.org
> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> index 04379db93571b..f2988659a5cbb 100644
> --- a/drivers/hid/hid-cp2112.c
> +++ b/drivers/hid/hid-cp2112.c
> @@ -1430,6 +1430,12 @@ static int cp2112_raw_event(struct hid_device *hdev, struct hid_report *report,
>
> switch (data[0]) {
> case CP2112_TRANSFER_STATUS_RESPONSE:
> + if (size < sizeof(*xfer)) {
> + dev->xfer_status = -EMSGSIZE;
> + atomic_set(&dev->xfer_avail, 1);
[Severity: High]
This isn't a bug introduced by this patch, but is it safe to use atomic_set()
here without a write memory barrier?
If a thread waiting on another CPU reads the flag locklessly via
wait_event_interruptible_timeout() in cp2112_wait(), could the CPU reorder
the data stores and loads? For example:
cp2112_wait()
ret = wait_event_interruptible_timeout(dev->wait,
atomic_read(avail), msecs_to_jiffies(RESPONSE_TIMEOUT));
Could this cause the reader to see the flag as set but read stale data?
[Severity: High]
This is also a pre-existing issue, but does setting the availability flag
unconditionally here leave us vulnerable to late USB responses?
If an I2C transfer times out and a new transfer is initiated, could a delayed
response from the aborted transfer arrive here and incorrectly wake up the new
transfer since there's no sequence number checking?
> + break;
> + }
> +
[ ... ]
> case CP2112_DATA_READ_RESPONSE:
> + if (size < 3 || data[2] > size - 3) {
> + dev->read_length = 0;
> + atomic_set(&dev->read_avail, 1);
> + break;
> + }
> +
While reviewing this file, I noticed a couple of other pre-existing issues:
[Severity: High]
This is a pre-existing issue, but does disabling a GPIO IRQ cause a system
panic by sleeping in atomic context?
The irqchip callback irq_shutdown is called by the IRQ core with the
per-IRQ desc->lock raw spinlock held. In cp2112_gpio_irq_shutdown():
cp2112_gpio_irq_shutdown()
cancel_delayed_work_sync(&dev->gpio_poll_worker);
Could calling this sleeping function inside a raw spinlock trigger a BUG
scheduling while atomic?
[Severity: High]
This is another pre-existing issue, but can concurrent masking and unmasking
corrupt dev->irq_mask?
The irq_mask and irq_unmask callbacks are called with the per-pin desc->lock
held, so they can execute concurrently on different CPUs for different pins.
In cp2112_gpio_irq_mask():
__clear_bit(hwirq, &dev->irq_mask);
And in cp2112_gpio_irq_unmask():
__set_bit(hwirq, &dev->irq_mask);
Do these non-atomic bitwise updates create a read-modify-write data race on
the shared dev->irq_mask, leading to lost or stuck interrupts?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628164731.17614-1-alhouseenyousef@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 3/6] HID: roccat-pyra: reject short button reports
From: sashiko-bot @ 2026-06-28 16:59 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628164611.17467-3-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free (UAF) in `pyra_raw_event` and `roccat_report_event` due to freeing device data before stopping the hardware.
- [Medium] Data race on `pyra->actual_profile` and `pyra->actual_cpi` between process context and atomic context.
- [Medium] Missing lock serialization across the multi-step device control sequence in sysfs read handlers.
- [Medium] The identical out-of-bounds read vulnerability fixed in `pyra` and other drivers in this series is still present in `savu`.
--
commit c58b9d043952189d7a69bf9c149768907653644a
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: roccat-pyra: reject short button reports
The Pyra raw-event path treats every button report as a complete
five-byte structure. A malformed USB device can send a shorter report
and make profile tracking or character-device event construction read
beyond the received input buffer.
> diff --git a/drivers/hid/hid-roccat-pyra.c b/drivers/hid/hid-roccat-pyra.c
> index 0d515995bb9d6..df8949c17ce3c 100644
> --- a/drivers/hid/hid-roccat-pyra.c
> +++ b/drivers/hid/hid-roccat-pyra.c
> @@ -557,6 +557,10 @@ static int pyra_raw_event(struct hid_device *hdev, struct hid_report *report,
> if (pyra == NULL)
> return 0;
[Severity: High]
This isn't a bug introduced by this patch, but is it possible for a
use-after-free to occur here during device removal?
In pyra_remove(), pyra_remove_specials() frees the device data via kfree()
before hid_hw_stop(hdev) is called. Because the hardware isn't stopped yet,
the USB interrupt endpoint remains active.
If an event is received in this narrow window, pyra_raw_event() fetches the
freed pointer locklessly. Since the pointer was never cleared, it passes
the NULL check and dereferences freed memory. Can we reorder the cleanup in
pyra_remove() to stop the hardware before freeing the data?
> + if (data[0] == PYRA_MOUSE_REPORT_NUMBER_BUTTON &&
> + size < sizeof(struct pyra_mouse_event_button))
> + return 0;
> +
> pyra_keep_values_up_to_date(pyra, data);
[Severity: Medium]
This is a pre-existing issue, but does pyra_keep_values_up_to_date() have a
data race with sysfs handlers?
The function pyra_sysfs_write_settings() runs in process context and modifies
shared state like pyra->actual_profile and pyra->actual_cpi under the
pyra->pyra_lock mutex. Meanwhile, pyra_raw_event() runs in atomic context and
modifies these same variables locklessly via pyra_keep_values_up_to_date().
An interrupt firing during a sysfs write could cause concurrent modifications,
leaving the variables in an inconsistent state.
> if (pyra->roccat_claimed)
> pyra_report_to_chrdev(pyra, data);
>
> return 0;
> }
[Severity: Medium]
This is also a pre-existing issue, but are the sysfs read handlers missing
lock serialization across multi-step device control sequences?
To read profile settings, handlers like pyra_sysfs_read_profilex_settings()
in drivers/hid/hid-roccat-pyra.c first send a control command via
pyra_send_control(), then read the data via pyra_sysfs_read(). The
pyra_send_control() call is unprotected, and pyra_sysfs_read() only acquires
pyra->pyra_lock for the receive operation.
If two threads read from different profiles concurrently, the send/receive
sequences can interleave (e.g., Thread 1 requests Profile 1, Thread 2 requests
Profile 2, Thread 1 receives data meant for Profile 2). Does the lock need to
cover the entire request-response transaction?
[Severity: Medium]
Finally, this is a pre-existing issue outside of the pyra driver, but does
the savu driver suffer from the same out-of-bounds read vulnerability being
fixed here?
While this patch adds size checks to pyra, savu_raw_event() in
drivers/hid/hid-roccat-savu.c still passes the unverified data buffer directly
to savu_report_to_chrdev(). The data is cast to struct
savu_mouse_report_special without ensuring it contains enough bytes.
If a malformed USB device sends a short report, it looks like this will leak
stale data from previous USB packets into the character device event stream.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628164611.17467-1-alhouseenyousef@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH 2/6] HID: roccat-isku: reject short button reports
From: sashiko-bot @ 2026-06-28 16:59 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628164611.17467-2-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] The `.raw_event` callback acquires mutexes while running in an atomic context, leading to a "sleeping function called from invalid context" bug. The HID core invokes the raw event handlers (`isku_raw_event`, etc.) from the USB URB completion handler, which executes in atomic context (softirq). The event handler delegates to `roccat_report_event()`, which erroneously acquires two mutexes.
- [High] Improper teardown ordering allows the URB completion handler to access freed driver data (Use-After-Free). In `isku_remove()` and the error path of `isku_probe()`, `isku_remove_specials()` is called before `hid_hw_stop()`. This frees the `isku` structure but leaves the hardware URB polling active. If a USB interrupt completes in this window, `isku_raw_event()` fetches the freed `isku` pointer via `hid_get_drvdata(hdev)` and dereferences it.
- [Medium] The patch fixes an out-of-bounds read in `isku_raw_event` caused by missing size validation, and subsequent patches in the series fix similar flaws in `koneplus`, `konepure`, `kovaplus`, `pyra`, and `ryos`. However, the `savu` driver was left unpatched. In `savu_raw_event()`, if a malformed USB device issues a truncated packet, the driver reads fields directly from the uninitialized portion of the HID core's URB buffer (out-of-bounds of the actual packet) and forwards this data to the character device, leaking kernel memory to user space.
--
commit c7e16c12dacea0adf121145e16f7eec60bf190b2
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: roccat-isku: reject short button reports
The Isku raw-event path casts button reports to a five-byte structure
and reads the event payload without validating the received size. A
malformed USB device can therefore trigger out-of-bounds reads from a
short report. Require the complete button report before updating or
forwarding it.
> diff --git a/drivers/hid/hid-roccat-isku.c b/drivers/hid/hid-roccat-isku.c
> index 93a49c93ae8c0..c65f414b13cdf 100644
> --- a/drivers/hid/hid-roccat-isku.c
> +++ b/drivers/hid/hid-roccat-isku.c
> @@ -411,6 +411,10 @@ static int isku_raw_event(struct hid_device *hdev,
> if (isku == NULL)
> return 0;
>
> + if (data[0] == ISKU_REPORT_NUMBER_BUTTON &&
> + size < sizeof(struct isku_report_button))
> + return 0;
> +
[Severity: Medium]
This isn't a bug introduced by this patch, but since we are fixing
out-of-bounds reads here, should a similar size validation be applied to the
savu driver?
In drivers/hid/hid-roccat-savu.c:savu_raw_event(), if a malformed USB device
issues a truncated packet, the driver passes input data without size
validation:
if (savu->roccat_claimed)
savu_report_to_chrdev(savu, data);
Then savu_report_to_chrdev() reads fields directly from the uninitialized
portion of the HID core's URB buffer:
special_report = (struct savu_mouse_report_special const *)data;
roccat_report.type = special_report->type;
Could this leak kernel memory to user space?
> isku_keep_values_up_to_date(isku, data);
[Severity: High]
This is a pre-existing issue, but does the teardown order in isku_remove()
allow a use-after-free here if an interrupt fires during device disconnect?
In isku_remove() and the probe error path, the driver data is freed via
isku_remove_specials() before stopping the hardware URBs:
isku_remove_specials(hdev);
hid_hw_stop(hdev);
If a final URB completes in this window, isku_raw_event() fetches the freed
isku pointer and dereferences it in isku_keep_values_up_to_date().
>
> if (isku->roccat_claimed)
[Severity: High]
This is a pre-existing issue, but does this call chain sleep in atomic context?
The HID core invokes isku_raw_event() from the USB URB completion handler
in softirq context. This delegates to isku_report_to_chrdev() and then to
roccat_report_event(), which acquires mutexes:
drivers/hid/hid-roccat.c:roccat_report_event() {
...
mutex_lock(&device->readers_lock);
mutex_lock(&device->cbuf_lock);
...
}
Can this cause a "sleeping function called from invalid context" bug?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628164611.17467-1-alhouseenyousef@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH 5/6] HID: roccat-konepure: reject short button reports
From: sashiko-bot @ 2026-06-28 16:59 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628164611.17467-5-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] The patch series fixes out-of-bounds reads in several Roccat drivers by adding report size validation, but misses identical vulnerabilities in `hid-roccat-savu.c`.
- [High] The `roccat_report_event()` function illegally acquires sleeping mutex locks while executing in atomic/softirq context, leading to scheduling-while-atomic bugs.
- [High] Leaking `struct roccat_device` and `struct roccat_reader` in `roccat_release()` after device disconnect.
- [High] Leaking dynamically allocated report buffers in the ring buffer when destroying `roccat_device`.
- [High] Missing size checks in `corsair_void_raw_event()` allow out-of-bounds memory reads.
--
commit ed9829ff5a1328c7edd5ce7aa1cab27bdb994e83
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: roccat-konepure: reject short button reports
konepure_raw_event() forwards button reports directly to
roccat_report_event(), which copies the fixed eight-byte report size
registered by this driver. A malformed USB device can send a shorter
report and make that copy read beyond the input buffer.
> diff --git a/drivers/hid/hid-roccat-konepure.c b/drivers/hid/hid-roccat-konepure.c
> index 7f753dfc2a108..d17dee18ac2b0 100644
> --- a/drivers/hid/hid-roccat-konepure.c
> +++ b/drivers/hid/hid-roccat-konepure.c
> @@ -181,7 +181,8 @@ static int konepure_raw_event(struct hid_device *hdev,
> != USB_INTERFACE_PROTOCOL_MOUSE)
> return 0;
>
> - if (data[0] != KONEPURE_MOUSE_REPORT_NUMBER_BUTTON)
> + if (data[0] != KONEPURE_MOUSE_REPORT_NUMBER_BUTTON ||
> + size < sizeof(struct konepure_mouse_report_button))
> return 0;
>
> if (konepure != NULL && konepure->roccat_claimed)
> roccat_report_event(konepure->chrdev_minor, data);
[Severity: High]
This is a pre-existing issue, but does calling roccat_report_event() here
lead to scheduling while atomic?
The raw_event handler runs in softirq context when the URB completion
handler calls hid_input_report(). However, roccat_report_event() acquires
sleeping locks:
drivers/hid/hid-roccat.c:roccat_report_event() {
...
mutex_lock(&device->readers_lock);
mutex_lock(&device->cbuf_lock);
...
}
Will this cause lockdep warnings or panics when those mutexes sleep?
[Severity: High]
This isn't a bug introduced by this patch, but while adding report size
validation for konepure, did we miss identical vulnerabilities in
hid-roccat-savu.c?
Looking at savu_raw_event(), it lacks report size validation before
processing the raw event buffer:
drivers/hid/hid-roccat-savu.c:savu_raw_event() {
static int savu_raw_event(struct hid_device *hdev,
struct hid_report *report, u8 *data, int size)
...
}
Which then performs out-of-bounds struct field accesses on short reports:
drivers/hid/hid-roccat-savu.c:savu_report_to_chrdev() {
...
special_report = (struct savu_mouse_report_special const *)data;
roccat_report.type = special_report->type;
roccat_report.data[0] = special_report->data[0];
roccat_report.data[1] = special_report->data[1];
...
}
Could this lead to an out-of-bounds read if a malformed device sends a
shorter report, potentially leaking kernel memory to userspace?
[Severity: High]
This is a pre-existing issue, but is there a memory leak in roccat_release()
when bailing out early?
If a Roccat USB device is unplugged while a process has the character
device open, roccat_disconnect() sets devices[minor] = NULL but defers
freeing the device. Later, when the user closes it:
drivers/hid/hid-roccat.c:roccat_release() {
...
device = devices[minor];
if (!device) {
mutex_unlock(&devices_lock);
pr_emerg("roccat device with minor %d doesn't exist\n", minor);
return -ENODEV;
}
...
}
Does this early return skip freeing the reader struct and fail to decrement
device->open, permanently leaking both structures?
[Severity: High]
This is a pre-existing issue, but does destroying roccat_device leak
dynamically allocated report buffers?
When the device is finally destroyed, we free the device structure:
drivers/hid/hid-roccat.c:roccat_disconnect() {
...
} else {
kfree(device);
}
...
}
And similarly in roccat_release():
drivers/hid/hid-roccat.c:roccat_release() {
...
} else {
kfree(device);
}
...
}
Since roccat_report_event() allocates report data via kmemdup() into the
cbuf ring buffer, does calling kfree(device) without iterating over cbuf
leave those populated cbuf[i].value pointers leaked?
[Severity: High]
This isn't a bug introduced by this patch, but does a similar missing
size check exist in the Corsair VOID driver?
Looking at corsair_void_raw_event(), it processes status and firmware
reports by directly reading offsets up to data[4]:
drivers/hid/hid-corsair-void.c:corsair_void_raw_event() {
...
if (hid_report->id == CORSAIR_VOID_STATUS_REPORT_ID) {
drvdata->mic_up = FIELD_GET(CORSAIR_VOID_MIC_MASK, data[2]);
drvdata->connected = (data[3] == CORSAIR_VOID_WIRELESS_CONNECTED) ||
drvdata->is_wired;
...
}
Without verifying that size is large enough, could a short report cause
an out-of-bounds memory read?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628164611.17467-1-alhouseenyousef@gmail.com?part=5
^ permalink raw reply
* Re: [PATCH 4/6] HID: roccat-kovaplus: reject short button reports
From: sashiko-bot @ 2026-06-28 16:58 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628164611.17467-4-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing bounds check in savu_raw_event allows out-of-bounds read and kernel memory leak
--
commit e1ed8982777cb731a4bf22c7e12d12c6c15620cc
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: roccat-kovaplus: reject short button reports
The Kovaplus raw-event helpers cast button reports to a five-byte
structure and read all payload fields without checking the received size.
A malformed USB device can therefore trigger out-of-bounds reads from a
short input report.
> The Kovaplus raw-event helpers cast button reports to a five-byte
> structure and read all payload fields without checking the received size.
> A malformed USB device can therefore trigger out-of-bounds reads from a
> short input report.
[Severity: High]
This isn't a bug introduced by this patch, but it looks like the exact same
vulnerability exists in the savu driver.
While missing checks in several other roccat drivers (koneplus, isku, pyra,
konepure, ryos) were addressed in this series, it appears the savu driver
was missed.
In drivers/hid/hid-roccat-savu.c:savu_raw_event():
if (savu->roccat_claimed)
savu_report_to_chrdev(savu, data);
This calls savu_report_to_chrdev() without validating that size is sufficient
for struct savu_mouse_report_special.
Could a malicious physical USB device send a short HID report with size < 5
and data[0] == SAVU_MOUSE_REPORT_NUMBER_SPECIAL to trigger an out-of-bounds
read and expose uninitialized bytes from the HID transfer buffer to
userspace?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628164611.17467-1-alhouseenyousef@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH 4/4] HID: roccat-savu: reject short special reports
From: sashiko-bot @ 2026-06-28 16:49 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628163527.14279-4-alhouseenyousef@gmail.com>
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 <alhouseenyousef@gmail.com>
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.
> 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
[ ... ]
> @@ -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
^ permalink raw reply
* Re: [PATCH 2/4] HID: cougar: reject short vendor reports
From: sashiko-bot @ 2026-06-28 16:49 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260628163527.14279-2-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] `cougar_raw_event()` accesses `shared->input` without synchronization against `cougar_remove()`, leading to a potential Use-After-Free if the keyboard interface is unbound.
- [High] A deadlock occurs if `devm_add_action_or_reset()` fails during probe.
- [High] A race condition between `kref_put()` and list lookup leads to a Use-After-Free of `cougar_shared`.
--
commit 27f66ec84e2d69ee2a0830cfc45515ced43d2bb5
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: cougar: reject short vendor reports
This commit fixes an out-of-bounds read in cougar_raw_event() by ensuring
that vendor-interface reports are large enough before parsing them. Short
reports are now properly rejected with an error.
> diff --git a/drivers/hid/hid-cougar.c b/drivers/hid/hid-cougar.c
> index ad027c45f1628..7156658166f51 100644
> --- a/drivers/hid/hid-cougar.c
> +++ b/drivers/hid/hid-cougar.c
> @@ -270,6 +270,9 @@ static int cougar_raw_event(struct hid_device *hdev, struct hid_report *report,
> if (!shared->enabled || !shared->input)
> return -EPERM;
>
[Severity: High]
This is a pre-existing issue, but is cougar_raw_event() accessing shared->input
without synchronization against cougar_remove()?
If a thread is preempted right after checking shared->enabled and shared->input,
and cougar_remove() runs concurrently, it will set shared->enabled to false and
call hid_hw_stop(). This destroys and frees the input_dev structure.
When the first thread resumes, wouldn't it pass the freed shared->input to
input_event(), resulting in a Use-After-Free?
> + if (size <= COUGAR_FIELD_ACTION)
> + return -EPERM;
> +
> code = data[COUGAR_FIELD_CODE];
> action = data[COUGAR_FIELD_ACTION];
[ ... ]
While reviewing this file, I noticed a couple of other pre-existing issues in
functions outside of this patch.
drivers/hid/hid-cougar.c:cougar_bind_shared_data() {
...
mutex_lock(&cougar_udev_list_lock);
...
error = devm_add_action_or_reset(&hdev->dev, cougar_remove_shared_data, cougar);
...
}
[Severity: High]
This isn't a bug introduced by this patch, but could this create a deadlock if
devm_add_action_or_reset() fails during probe?
If it fails, it synchronously executes the cleanup callback
cougar_remove_shared_data(), which calls kref_put(). If the refcount drops to 0,
cougar_release_shared_data() is called and attempts to acquire
cougar_udev_list_lock. Since that lock is already held by
cougar_bind_shared_data(), this would cause a deadlock.
drivers/hid/hid-cougar.c:cougar_remove_shared_data() {
...
if (cougar->shared) {
kref_put(&cougar->shared->kref, cougar_release_shared_data);
cougar->shared = NULL;
}
...
}
[Severity: High]
This is also a pre-existing issue, but might there be a race condition between
kref_put() and the list lookup?
The refcount is decremented to zero without holding cougar_udev_list_lock.
Before the object is removed from the list in cougar_release_shared_data(), a
concurrent call to cougar_get_shared_data() might iterate the list and use
kref_get() on this object while it is pending deletion.
Would using kref_get_unless_zero() during the lookup, or holding the list lock
during kref_put(), prevent this Use-After-Free?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628163527.14279-1-alhouseenyousef@gmail.com?part=2
^ permalink raw reply
* [PATCH] HID: cp2112: validate input response lengths
From: Yousef Alhouseen @ 2026-06-28 16:47 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, stable, Yousef Alhouseen
cp2112_raw_event() parses transfer-status responses as a fixed seven-byte
structure without checking the report size. It also trusts the length in
data-read responses and copies that many bytes even when the received
report is shorter. A malformed USB device can use either path to trigger
out-of-bounds reads from the HID input buffer.
Reject short status responses with -EMSGSIZE. Treat truncated data
responses as zero-length reads so the waiting transfer fails instead of
timing out or copying beyond the report.
Fixes: e932d8178667 ("HID: add hid-cp2112 driver")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-cp2112.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 04379db93571..f2988659a5cb 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1430,6 +1430,12 @@ static int cp2112_raw_event(struct hid_device *hdev, struct hid_report *report,
switch (data[0]) {
case CP2112_TRANSFER_STATUS_RESPONSE:
+ if (size < sizeof(*xfer)) {
+ dev->xfer_status = -EMSGSIZE;
+ atomic_set(&dev->xfer_avail, 1);
+ break;
+ }
+
hid_dbg(hdev, "xfer status: %02x %02x %04x %04x\n",
xfer->status0, xfer->status1,
be16_to_cpu(xfer->retries), be16_to_cpu(xfer->length));
@@ -1463,6 +1469,12 @@ static int cp2112_raw_event(struct hid_device *hdev, struct hid_report *report,
atomic_set(&dev->xfer_avail, 1);
break;
case CP2112_DATA_READ_RESPONSE:
+ if (size < 3 || data[2] > size - 3) {
+ dev->read_length = 0;
+ atomic_set(&dev->read_avail, 1);
+ break;
+ }
+
hid_dbg(hdev, "read response: %02x %02x\n", data[1], data[2]);
dev->read_length = data[2];
@@ -1494,4 +1506,3 @@ module_hid_driver(cp2112_driver);
MODULE_DESCRIPTION("Silicon Labs HID USB to SMBus master bridge");
MODULE_AUTHOR("David Barksdale <dbarksdale@uplogix.com>");
MODULE_LICENSE("GPL");
-
--
2.54.0
^ permalink raw reply related
* [PATCH 6/6] HID: roccat-ryos: reject short special reports
From: Yousef Alhouseen @ 2026-06-28 16:46 UTC (permalink / raw)
To: Stefan Achatz, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, stable, Yousef Alhouseen
In-Reply-To: <20260628164611.17467-1-alhouseenyousef@gmail.com>
ryos_raw_event() forwards special reports directly to the Roccat
character-device layer, which copies the fixed five-byte report size
registered by this driver. A malformed USB device can send a shorter
report and trigger an out-of-bounds read during that copy.
Only forward complete special reports.
Fixes: 6f3a19360545 ("HID: roccat: add support for Ryos MK keyboards")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-roccat-ryos.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-roccat-ryos.c b/drivers/hid/hid-roccat-ryos.c
index db83f42457da..5087defc7c37 100644
--- a/drivers/hid/hid-roccat-ryos.c
+++ b/drivers/hid/hid-roccat-ryos.c
@@ -189,7 +189,8 @@ static int ryos_raw_event(struct hid_device *hdev,
!= RYOS_USB_INTERFACE_PROTOCOL)
return 0;
- if (data[0] != RYOS_REPORT_NUMBER_SPECIAL)
+ if (data[0] != RYOS_REPORT_NUMBER_SPECIAL ||
+ size < sizeof(struct ryos_report_special))
return 0;
if (ryos != NULL && ryos->roccat_claimed)
--
2.54.0
^ permalink raw reply related
* [PATCH 5/6] HID: roccat-konepure: reject short button reports
From: Yousef Alhouseen @ 2026-06-28 16:46 UTC (permalink / raw)
To: Stefan Achatz, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, stable, Yousef Alhouseen
In-Reply-To: <20260628164611.17467-1-alhouseenyousef@gmail.com>
konepure_raw_event() forwards button reports directly to
roccat_report_event(), which copies the fixed eight-byte report size
registered by this driver. A malformed USB device can send a shorter
report and make that copy read beyond the input buffer.
Only forward complete button reports.
Fixes: 8936aa31cd5f ("HID: roccat: add support for Roccat Kone Pure gaming mouse")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-roccat-konepure.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-roccat-konepure.c b/drivers/hid/hid-roccat-konepure.c
index 7f753dfc2a10..d17dee18ac2b 100644
--- a/drivers/hid/hid-roccat-konepure.c
+++ b/drivers/hid/hid-roccat-konepure.c
@@ -181,7 +181,8 @@ static int konepure_raw_event(struct hid_device *hdev,
!= USB_INTERFACE_PROTOCOL_MOUSE)
return 0;
- if (data[0] != KONEPURE_MOUSE_REPORT_NUMBER_BUTTON)
+ if (data[0] != KONEPURE_MOUSE_REPORT_NUMBER_BUTTON ||
+ size < sizeof(struct konepure_mouse_report_button))
return 0;
if (konepure != NULL && konepure->roccat_claimed)
--
2.54.0
^ permalink raw reply related
* [PATCH 4/6] HID: roccat-kovaplus: reject short button reports
From: Yousef Alhouseen @ 2026-06-28 16:46 UTC (permalink / raw)
To: Stefan Achatz, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, stable, Yousef Alhouseen
In-Reply-To: <20260628164611.17467-1-alhouseenyousef@gmail.com>
The Kovaplus raw-event helpers cast button reports to a five-byte
structure and read all payload fields without checking the received size.
A malformed USB device can therefore trigger out-of-bounds reads from a
short input report.
Require a complete button report before updating or forwarding it.
Fixes: 0e70f97f257e ("HID: roccat: Add support for Kova[+] mouse")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-roccat-kovaplus.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-roccat-kovaplus.c b/drivers/hid/hid-roccat-kovaplus.c
index 9ec42c218ef9..55de262e165b 100644
--- a/drivers/hid/hid-roccat-kovaplus.c
+++ b/drivers/hid/hid-roccat-kovaplus.c
@@ -614,6 +614,10 @@ static int kovaplus_raw_event(struct hid_device *hdev,
if (kovaplus == NULL)
return 0;
+ if (data[0] == KOVAPLUS_MOUSE_REPORT_NUMBER_BUTTON &&
+ size < sizeof(struct kovaplus_mouse_report_button))
+ return 0;
+
kovaplus_keep_values_up_to_date(kovaplus, data);
if (kovaplus->roccat_claimed)
--
2.54.0
^ permalink raw reply related
* [PATCH 3/6] HID: roccat-pyra: reject short button reports
From: Yousef Alhouseen @ 2026-06-28 16:46 UTC (permalink / raw)
To: Stefan Achatz, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, stable, Yousef Alhouseen
In-Reply-To: <20260628164611.17467-1-alhouseenyousef@gmail.com>
The Pyra raw-event path treats every button report as a complete
five-byte structure. A malformed USB device can send a shorter report
and make profile tracking or character-device event construction read
beyond the received input buffer.
Ignore incomplete button reports before calling either helper.
Fixes: cb7cf3da0daa ("HID: roccat: add driver for Roccat Pyra mouse")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-roccat-pyra.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-roccat-pyra.c b/drivers/hid/hid-roccat-pyra.c
index 0d515995bb9d..df8949c17ce3 100644
--- a/drivers/hid/hid-roccat-pyra.c
+++ b/drivers/hid/hid-roccat-pyra.c
@@ -557,6 +557,10 @@ static int pyra_raw_event(struct hid_device *hdev, struct hid_report *report,
if (pyra == NULL)
return 0;
+ if (data[0] == PYRA_MOUSE_REPORT_NUMBER_BUTTON &&
+ size < sizeof(struct pyra_mouse_event_button))
+ return 0;
+
pyra_keep_values_up_to_date(pyra, data);
if (pyra->roccat_claimed)
--
2.54.0
^ permalink raw reply related
* [PATCH 2/6] HID: roccat-isku: reject short button reports
From: Yousef Alhouseen @ 2026-06-28 16:46 UTC (permalink / raw)
To: Stefan Achatz, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, stable, Yousef Alhouseen
In-Reply-To: <20260628164611.17467-1-alhouseenyousef@gmail.com>
The Isku raw-event path casts button reports to a five-byte structure
and reads the event payload without validating the received size. A
malformed USB device can therefore trigger out-of-bounds reads from a
short report.
Require the complete button report before updating or forwarding it.
Fixes: d41c2a7011df ("HID: roccat: Add support for Isku keyboard")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-roccat-isku.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-roccat-isku.c b/drivers/hid/hid-roccat-isku.c
index 93a49c93ae8c..c65f414b13cd 100644
--- a/drivers/hid/hid-roccat-isku.c
+++ b/drivers/hid/hid-roccat-isku.c
@@ -411,6 +411,10 @@ static int isku_raw_event(struct hid_device *hdev,
if (isku == NULL)
return 0;
+ if (data[0] == ISKU_REPORT_NUMBER_BUTTON &&
+ size < sizeof(struct isku_report_button))
+ return 0;
+
isku_keep_values_up_to_date(isku, data);
if (isku->roccat_claimed)
--
2.54.0
^ permalink raw reply related
* [PATCH 1/6] HID: roccat-koneplus: reject short button reports
From: Yousef Alhouseen @ 2026-06-28 16:46 UTC (permalink / raw)
To: Stefan Achatz, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, stable, Yousef Alhouseen
The Koneplus raw-event path casts button reports to an eight-byte
structure before updating profile state and forwarding an event. A
malformed USB device can identify a shorter report as a button report
and trigger out-of-bounds reads.
Require the complete button report before either consumer sees it.
Fixes: 47dbdbffe15b ("HID: roccat: Add support for Roccat Kone[+] v2")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-roccat-koneplus.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-roccat-koneplus.c b/drivers/hid/hid-roccat-koneplus.c
index f80a60539a96..e0f35251e81a 100644
--- a/drivers/hid/hid-roccat-koneplus.c
+++ b/drivers/hid/hid-roccat-koneplus.c
@@ -523,6 +523,10 @@ static int koneplus_raw_event(struct hid_device *hdev,
if (koneplus == NULL)
return 0;
+ if (data[0] == KONEPLUS_MOUSE_REPORT_NUMBER_BUTTON &&
+ size < sizeof(struct koneplus_mouse_report_button))
+ return 0;
+
koneplus_keep_values_up_to_date(koneplus, data);
if (koneplus->roccat_claimed)
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] HID: mcp2200: reject short read-all responses
From: sashiko-bot @ 2026-06-28 16:42 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628163035.12212-1-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing synchronization in `mcp_cmd_read_all` and lockless shared state access in `mcp2200_raw_event` lead to race conditions, timeouts, and data corruption.
- [High] `mcp_get` silently ignores negative error codes returned by `mcp_get_multiple`, converting hardware or communication errors into a valid 'low' state.
- [High] The driver fails to synchronize its internal state cache (`mcp->gpio_dir` and `mcp->gpio_val`) with the actual hardware configuration during probe.
- [High] Device unbind race condition leading to Use-After-Free due to improper devm_ teardown order.
- [High] The `hid_report` buffer is embedded directly within `struct mcp2200` and passed to DMA mapping functions, violating DMA cache-line alignment requirements.
--
commit 3df6b9c6c4d5763dc3316160e66ba44dfef6c606
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: mcp2200: reject short read-all responses
This commit adds a size check for READ_ALL responses in the MCP2200
driver. If the response size is smaller than the expected struct size,
it sets the status to -EMSGSIZE and aborts processing to prevent
out-of-bounds reads.
> diff --git a/drivers/hid/hid-mcp2200.c b/drivers/hid/hid-mcp2200.c
> index dafdd5b4a0797..d49f3aa44448e 100644
> --- a/drivers/hid/hid-mcp2200.c
> +++ b/drivers/hid/hid-mcp2200.c
> @@ -302,6 +302,10 @@ static int mcp2200_raw_event(struct hid_device *hdev, struct hid_report *report,
> switch (data[0]) {
> case READ_ALL:
> all_resp = (struct mcp_read_all_resp *) data;
> + if (size < sizeof(*all_resp)) {
> + mcp->status = -EMSGSIZE;
[Severity: High]
This is a pre-existing issue, but won't this new -EMSGSIZE error be silently
swallowed by mcp_get()?
Looking at mcp_get(), it calls mcp_get_multiple() which propagates this
negative error code. However, mcp_get() discards the return value:
drivers/hid/hid-mcp2200.c:mcp_get() {
...
mcp_get_multiple(gc, &mask, &bits);
return bits > 0;
}
Since bits remains uninitialized or zeroed upon error, this converts the
-EMSGSIZE error (or any other communication failure) into a valid logical
low state.
Does this need to be handled so downstream drivers don't receive false
logical states when the device errors out?
> + break;
> + }
> mcp->status = 0;
> mcp->gpio_inval = all_resp->io_port_val_bmap;
[Severity: High]
This is a pre-existing issue, but does mcp2200_raw_event() update shared
device state locklessly?
mcp_cmd_read_all() orchestrates a transaction but drops mcp->lock before
waiting for a completion, and calls reinit_completion() unprotected:
drivers/hid/hid-mcp2200.c:mcp_cmd_read_all() {
...
reinit_completion(&mcp->wait_in_report);
mutex_lock(&mcp->lock);
...
mutex_unlock(&mcp->lock);
...
t = wait_for_completion_timeout(&mcp->wait_in_report, ...);
}
If multiple threads concurrently invoke mcp_cmd_read_all(), or unrelated
hardware reports arrive while waiting, their wait queues and completion
states can collide.
Furthermore, mcp2200_raw_event() updates mcp->status and mcp->gpio_inval
locklessly in interrupt context, and unconditionally completes the wait queue
for any unexpected report.
Should a single lock serialize the entire command-response wait cycle?
[Severity: High]
This is also a pre-existing issue, but why is the io_bmap hardware direction
state provided in READ_ALL responses completely ignored here instead of being
saved to mcp->gpio_dir?
In mcp2200_probe(), mcp->gpio_dir defaults to 0 (all outputs) without
querying the initial state. Because io_bmap is ignored here, when
mcp_set_direction() is later called to modify a specific pin, it masks
against the uninitialized local cache:
drivers/hid/hid-mcp2200.c:mcp_set_direction() {
...
conf->io_bmap = (mcp->gpio_dir & ~(1 << gpio_nr))
| (io_direction << gpio_nr);
}
Doesn't this clobber the direction of all other pins, potentially causing
electrical short-circuits by driving input pins as outputs?
> mcp->baud_h = all_resp->baud_h;
[ ... ]
Additionally, there are a couple of other pre-existing issues in this driver
that might be worth addressing:
[Severity: High]
This is a pre-existing issue, but is there a device unbind race condition
leading to a use-after-free due to the devm_ teardown order?
mcp2200_probe() uses devm_gpiochip_add_data() to register the gpiochip:
drivers/hid/hid-mcp2200.c:mcp2200_probe() {
...
ret = devm_gpiochip_add_data(&hdev->dev, &mcp->gc, mcp);
...
}
However, mcp2200_remove() calls hid_hw_stop(hdev) synchronously, freeing
underlying URBs:
drivers/hid/hid-mcp2200.c:mcp2200_remove() {
...
hid_hw_stop(hdev);
}
Because devm_ cleanup callbacks run after mcp2200_remove() completes, the
gpiochip remains fully active.
If userspace invokes a GPIO operation during this window, mcp_cmd_read_all()
will call hid_hw_output_report(), triggering a use-after-free on the freed
urbout.
Should the driver use manual teardown in remove() instead of devm_?
[Severity: High]
This is a pre-existing issue, but does the hid_report buffer violate DMA
cache-line alignment requirements?
The hid_report buffer is embedded directly within struct mcp2200:
drivers/hid/hid-mcp2200.c:
struct mcp2200 {
...
struct gpio_chip gc;
u8 hid_report[16];
};
This buffer is placed at the end of the struct without a
____cacheline_aligned attribute.
When hid_hw_output_report() is called, the USB HID layer receives this
buffer and maps it for DMA. On non-coherent architectures, mapping a buffer
that shares cache lines with other actively modified struct members (like gc
or status) can cause cache invalidation or flush operations to overwrite
concurrent CPU writes.
Could this buffer be allocated independently or aligned to the cacheline size?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628163035.12212-1-alhouseenyousef@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH] HID: pxrc: reject short input reports
From: sashiko-bot @ 2026-06-28 16:41 UTC (permalink / raw)
To: Yousef Alhouseen; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628162806.10675-1-alhouseenyousef@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete fix / logic error: returning 0 for short reports does not drop the report.
--
commit 601e7ac29ed99f9b83b5ad4f7ade423528b08e79
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>
HID: pxrc: reject short input reports
This commit fixes an out-of-bounds access in pxrc_raw_event() by checking
if the incoming input report is shorter than 8 bytes and returning early.
> diff --git a/drivers/hid/hid-pxrc.c b/drivers/hid/hid-pxrc.c
> index 71fe0c06ddcdf..e3755d8b85c22 100644
> --- a/drivers/hid/hid-pxrc.c
> +++ b/drivers/hid/hid-pxrc.c
> @@ -55,6 +55,9 @@ static int pxrc_raw_event(struct hid_device *hdev, struct hid_report *report,
> {
> struct pxrc_priv *priv = hid_get_drvdata(hdev);
>
> + if (size < 8)
> + return 0;
[Severity: Medium]
Does returning 0 here achieve the goal of ignoring the report?
When the driver receives a short report and pxrc_raw_event() returns 0,
it signals success to the HID core. Looking at __hid_input_report(),
if the raw_event callback returns >= 0, the core falls through to
hid_report_raw_event(). This will zero-pad the missing bytes and dispatch
the report, which can result in erratic zero-values for the missing axes
in userspace applications.
Should this return a negative error code (like -EINVAL) to ensure the
malformed report is completely dropped by the core instead of padded?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628162806.10675-1-alhouseenyousef@gmail.com?part=1
^ permalink raw reply
* [PATCH 4/4] HID: roccat-savu: reject short special reports
From: Yousef Alhouseen @ 2026-06-28 16:35 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Stefan Achatz, linux-input, linux-kernel, stable,
Yousef Alhouseen
In-Reply-To: <20260628163527.14279-1-alhouseenyousef@gmail.com>
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.
Fixes: 6a2a6390cf09 ("HID: roccat: add support for Roccat Savu")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-roccat-savu.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-roccat-savu.c b/drivers/hid/hid-roccat-savu.c
index 679136933560..04fa4c50cfa4 100644
--- a/drivers/hid/hid-roccat-savu.c
+++ b/drivers/hid/hid-roccat-savu.c
@@ -152,12 +152,13 @@ static void savu_remove(struct hid_device *hdev)
}
static void savu_report_to_chrdev(struct roccat_common2_device const *savu,
- u8 const *data)
+ u8 const *data, int size)
{
struct savu_roccat_report roccat_report;
struct savu_mouse_report_special const *special_report;
- if (data[0] != SAVU_MOUSE_REPORT_NUMBER_SPECIAL)
+ if (data[0] != SAVU_MOUSE_REPORT_NUMBER_SPECIAL ||
+ size < sizeof(*special_report))
return;
special_report = (struct savu_mouse_report_special const *)data;
@@ -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);
return 0;
}
--
2.54.0
^ permalink raw reply related
* [PATCH 3/4] HID: zydacron: reject short key reports
From: Yousef Alhouseen @ 2026-06-28 16:35 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Stefan Achatz, linux-input, linux-kernel, stable,
Yousef Alhouseen
In-Reply-To: <20260628163527.14279-1-alhouseenyousef@gmail.com>
The raw-event handler reads data[1] for report IDs 2 and 3 without
checking that the report includes a payload byte. A malformed USB device
can submit a report containing only the report ID and trigger an
out-of-bounds read.
Ignore the key value when the payload byte is missing.
Fixes: d0742abaa1c3 ("HID: add omitted hid-zydacron.c file")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-zydacron.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hid/hid-zydacron.c b/drivers/hid/hid-zydacron.c
index 1aae80f848f5..b882d2a0ba1a 100644
--- a/drivers/hid/hid-zydacron.c
+++ b/drivers/hid/hid-zydacron.c
@@ -129,6 +129,9 @@ static int zc_raw_event(struct hid_device *hdev, struct hid_report *report,
switch (report->id) {
case 0x02:
case 0x03:
+ if (size < 2)
+ break;
+
switch (data[1]) {
case 0x10:
key = KEY_MODE;
--
2.54.0
^ permalink raw reply related
* [PATCH 2/4] HID: cougar: reject short vendor reports
From: Yousef Alhouseen @ 2026-06-28 16:35 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Stefan Achatz, linux-input, linux-kernel, stable,
Yousef Alhouseen
In-Reply-To: <20260628163527.14279-1-alhouseenyousef@gmail.com>
cougar_raw_event() reads the key code and action from bytes one and two
of every enabled vendor-interface report. A malformed USB device can
send a shorter report and make both accesses run beyond the received
buffer.
Consume short vendor reports without parsing them.
Fixes: b8e759b8f6da ("HID: cougar: Add support for the Cougar 500k Gaming Keyboard")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-cougar.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hid/hid-cougar.c b/drivers/hid/hid-cougar.c
index ad027c45f162..7156658166f5 100644
--- a/drivers/hid/hid-cougar.c
+++ b/drivers/hid/hid-cougar.c
@@ -270,6 +270,9 @@ static int cougar_raw_event(struct hid_device *hdev, struct hid_report *report,
if (!shared->enabled || !shared->input)
return -EPERM;
+ if (size <= COUGAR_FIELD_ACTION)
+ return -EPERM;
+
code = data[COUGAR_FIELD_CODE];
action = data[COUGAR_FIELD_ACTION];
for (i = 0; cougar_mapping[i][0]; i++) {
--
2.54.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox