[PATCH 1/6] HID: roccat-koneplus: reject short button reports

[PATCH 1/6] HID: roccat-koneplus: reject short button reports

[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

[PATCH 2/6] HID: roccat-isku: reject short button reports

[Yousef Alhouseen]

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

Re: [PATCH 2/6] HID: roccat-isku: reject short button reports

[sashiko-bot]

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

[PATCH 3/6] HID: roccat-pyra: reject short button reports

[Yousef Alhouseen]

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

Re: [PATCH 3/6] HID: roccat-pyra: reject short button reports

[sashiko-bot]

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

[PATCH 4/6] HID: roccat-kovaplus: reject short button reports

[Yousef Alhouseen]

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

Re: [PATCH 4/6] HID: roccat-kovaplus: reject short button reports

[sashiko-bot]

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

[PATCH 5/6] HID: roccat-konepure: reject short button reports

[Yousef Alhouseen]

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

Re: [PATCH 5/6] HID: roccat-konepure: reject short button reports

[sashiko-bot]

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

[PATCH 6/6] HID: roccat-ryos: reject short special reports

[Yousef Alhouseen]

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

Re: [PATCH 6/6] HID: roccat-ryos: reject short special reports

[sashiko-bot]

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

Re: [PATCH 1/6] HID: roccat-koneplus: reject short button reports

[sashiko-bot]

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