* Re: [PATCH] HID: intel-thc-hid: intel-quickspi: validate report size before copy
From: sashiko-bot @ 2026-06-28 13:51 UTC (permalink / raw)
To: HyeongJun An; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260628133717.941389-1-sammiee5311@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] The newly introduced bounds check rejects valid, maximum-sized output reports.
Pre-existing issues:
- [Critical] Heap buffer overflow in GET_REPORT requests due to ignored user space length parameter.
- [High] Data races and waitqueue corruption due to concurrent lockless HID requests.
- [High] Heap buffer overflow in incoming report processing due to missing buffer bounds check.
- [High] Heap buffer overflow in `read_dma_buffer()` due to 4K max packet size rounding.
--
commit c642b7d6dc654ce88acbb51328c011ab2d5e04f0
Author: HyeongJun An <sammiee5311@gmail.com>
HID: intel-thc-hid: intel-quickspi: validate report size before copy
write_cmd_to_txdma() builds an output report in qsdev->report_buf, a heap
buffer allocated in quickspi_alloc_report_buf() to the device-descriptor
derived max_report_len. This patch records the report_buf allocation size
and rejects reports that do not fit before copying.
> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> index 4ae2e1718b30c..1695efd5961d7 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> @@ -559,6 +559,8 @@ static int quickspi_alloc_report_buf(struct quickspi_device *qsdev)
> if (!qsdev->report_buf)
> return -ENOMEM;
>
> + qsdev->report_buf_size = max_report_len;
> +
[Severity: High]
Since this size is based on the unadjusted device descriptor maximums, will
this reject valid maximum-sized output reports?
When the driver builds an output report in write_cmd_to_txdma(), it prepends
HIDSPI_OUTPUT_REPORT_HEADER_SIZE. If a maximum-sized report is sent, the
combined size will exceed qsdev->report_buf_size and trigger the new bounds
check.
> return 0;
> }
>
[ ... ]
> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> index cb19057f1191b..db6054843e777 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> @@ -30,6 +30,9 @@ static int write_cmd_to_txdma(struct quickspi_device *qsdev,
>
> write_buf = (struct output_report *)qsdev->report_buf;
>
> + if (HIDSPI_OUTPUT_REPORT_SIZE(report_buf_len) > qsdev->report_buf_size)
> + return -EINVAL;
> +
> write_buf->output_hdr.report_type = report_type;
> write_buf->output_hdr.content_len = cpu_to_le16(report_buf_len);
> write_buf->output_hdr.content_id = report_id;
>
> if (report_buf && report_buf_len > 0)
> memcpy(write_buf->content, report_buf, report_buf_len);
[Severity: High]
This is a pre-existing issue, but can multiple threads corrupt
qsdev->report_buf by executing this concurrently?
Since HID core does not serialize raw_request, multiple threads performing
concurrent HID ioctls (like HIDIOCGFEATURE and HIDIOCSFEATURE) could enter
write_cmd_to_txdma() simultaneously. Without locking, they would overwrite the
shared qsdev->report_buf with their respective headers and payloads.
This concurrent access could also corrupt the shared waitqueue state when
handling the responses:
drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c:quickspi_handle_input_data() {
...
qsdev->get_report_cmpl = true;
wake_up_interruptible(&qsdev->get_report_cmpl_wq);
...
}
[Severity: Critical]
This is a pre-existing issue, but does quickspi_get_report() overflow the
caller's buffer during GET_REPORT requests?
When a user space process issues a GET_REPORT ioctl with a smaller buffer,
hid_hw_raw_request() allocates a kernel buffer matching the requested length.
However, quickspi_hid_raw_request() discards this length parameter:
drivers/hid/intel-thc-hid/intel-quickspi/quickspi-hid.c:quickspi_hid_raw_request() {
...
ret = quickspi_get_report(qsdev, rtype, reportnum, buf);
...
}
Then quickspi_get_report() copies the full device-supplied response into this
potentially smaller buffer:
drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c:quickspi_get_report() {
...
memcpy(buf, qsdev->report_buf, qsdev->report_len);
...
}
[Severity: High]
This is a pre-existing issue, but does quickspi_handle_input_data() lack
bounds checking before copying incoming reports?
When a device responds to a GET_FEATURE or GET_INPUT_REPORT, the function
trusts the incoming input_len:
drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c:quickspi_handle_input_data() {
...
qsdev->report_len = sizeof(body_hdr->content_id) + input_len;
input_report = input_body->content - sizeof(body_hdr->content_id);
memcpy(qsdev->report_buf, input_report, qsdev->report_len);
...
}
If a compromised or malicious SPI device sends an artificially large input_len
that fits within the DMA transaction's packet bounds (up to 4K) but exceeds
qsdev->report_buf_size, this memcpy will overflow the strictly allocated
qsdev->report_buf.
[Severity: High]
This is a pre-existing issue, but can the 4K packet size rounding in the DMA
configuration lead to a heap overflow?
dma_set_max_packet_size() rounds up the requested DMA max packet sizes (such
as max_input_len) to a 4K boundary:
drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:dma_set_max_packet_size() {
...
dma_config->max_packet_size = ALIGN(size, SZ_4K);
...
}
When read_dma_buffer() receives data, it bounds-checks mes_len against this
4K-aligned max_packet_size before copying into the caller's buffer:
drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c:read_dma_buffer() {
...
if (mes_len > read_config->max_packet_size) {
...
}
ret = sg_copy_to_buffer(sg, nent, read_buff, mes_len);
...
}
If a malicious device returns more data than max_input_len but less than the
4K boundary, the check passes. Since qsdev->input_buf was allocated without
this 4K alignment, the copy will overflow the buffer.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260628133717.941389-1-sammiee5311@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH] HID: steam: reject short serial number reports
From: Yousef Alhouseen @ 2026-06-28 16:23 UTC (permalink / raw)
To: Vicki Pfau, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, stable, syzbot+75f3f9bff8c510602d36
In-Reply-To: <612b5987-1bc6-4b42-bfba-9c72ee5d51dc@endrift.com>
Hi Vicki,
That broader fix sounds preferable. Please go ahead and split it out
and submit it; I will drop this version to avoid overlapping work.
Thanks,
Yousef
On Sat, 27 Jun 2026 17:47:40 -0700, Vicki Pfau <vi@endrift.com> wrote:
> Hi Yousef,
>
> On 6/27/26 5:41 PM, Yousef Alhouseen wrote:
> > steam_recv_report() may return a short positive response and copies
> > only the bytes actually received. steam_get_serial() nevertheless reads
> > the full three-byte header and trusts its length without checking that
> > the serial payload was returned.
> >
> > A malformed USB device can therefore make the driver read uninitialized
> > stack bytes. With a complete-looking short header, those bytes can also
> > be copied into steam->serial_no and printed.
> >
> > Account for the stripped report ID in the return value and reject replies
> > that do not contain both the header and its declared payload.
> >
> > Fixes: c164d6abf384 ("HID: add driver for Valve Steam Controller")
> > Reported-by: syzbot+75f3f9bff8c510602d36@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=75f3f9bff8c510602d36
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
> > ---
> > drivers/hid/hid-steam.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> > index 197126d6e081..8c8bfb10e8b8 100644
> > --- a/drivers/hid/hid-steam.c
> > +++ b/drivers/hid/hid-steam.c
> > @@ -454,11 +454,20 @@ static int steam_get_serial(struct steam_device *steam)
> > ret = steam_recv_report(steam, reply, sizeof(reply));
> > if (ret < 0)
> > goto out;
> > + /* hid_hw_raw_request() counts the stripped report ID byte. */
> > + if (ret < 4) {
> > + ret = -EIO;
> > + goto out;
> > + }
> > if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] < 1 ||
> > reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
> > ret = -EIO;
> > goto out;
> > }
> > + if (ret - 1 < 3 + reply[1]) {
> > + ret = -EIO;
> > + goto out;
> > + }
> > reply[3 + STEAM_SERIAL_LEN] = 0;
> > strscpy(steam->serial_no, reply + 3, reply[1]);
> > out:
>
> I already have locally a patch that fixes this as part of my pending Steam Controller 2 support. However, it chooses to fix it in a different way that would affect all uses of steam_recv_report instead of per-callsite (with only one callsite fixed). I am hoping to get this patchset submitted soon, once more widescale testing is done, but if you want in the meantime I can pull out that single fix and submit it separately; it's a bit more sprawling and involves adding a new function for combined send/recv.
>
> Vicki
^ permalink raw reply
* [PATCH] HID: pxrc: reject short input reports
From: Yousef Alhouseen @ 2026-06-28 16:28 UTC (permalink / raw)
To: Marcus Folkesson, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, stable, Yousef Alhouseen
pxrc_raw_event() unconditionally reads and writes data[7], although a
malformed USB device can submit a shorter input report. The raw-event
callback runs before the HID core expands short reports to the size from
the report descriptor, so this accesses beyond the received buffer.
Ignore reports that do not contain all eight controller axes.
Fixes: acc3e34613da ("HID: Add driver for PhoenixRC Flight Controller")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-pxrc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hid/hid-pxrc.c b/drivers/hid/hid-pxrc.c
index 71fe0c06ddcd..e3755d8b85c2 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;
+
if (priv->alternate)
priv->slider = data[7];
else
--
2.54.0
^ permalink raw reply related
* [PATCH] HID: mcp2200: reject short read-all responses
From: Yousef Alhouseen @ 2026-06-28 16:30 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, stable, Yousef Alhouseen
mcp2200_raw_event() casts every READ_ALL response to the full 16-byte
response structure and reads fields through byte 10 without checking the
received report size. A malformed USB device can therefore trigger
out-of-bounds reads from the input buffer.
Complete the pending command with -EMSGSIZE when the response is short.
Fixes: 740329d7120f ("HID: mcp2200: added driver for GPIOs of MCP2200")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-mcp2200.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-mcp2200.c b/drivers/hid/hid-mcp2200.c
index dafdd5b4a079..d49f3aa44448 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;
+ break;
+ }
mcp->status = 0;
mcp->gpio_inval = all_resp->io_port_val_bmap;
mcp->baud_h = all_resp->baud_h;
--
2.54.0
^ permalink raw reply related
* [PATCH 1/4] HID: elo: ignore short touch 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
elo_process_data() reads coordinates, flags, and pressure through data[7].
The raw-event callback only checks the packet marker, so a malformed USB
device can submit a one-byte marker report and trigger out-of-bounds
reads from the input buffer.
Only process touch packets that contain all eight protocol bytes.
Fixes: d23efc19478a ("HID: add driver for ELO 4000/4500")
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
drivers/hid/hid-elo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-elo.c b/drivers/hid/hid-elo.c
index b8f5f3eb53a4..1aeec712c67b 100644
--- a/drivers/hid/hid-elo.c
+++ b/drivers/hid/hid-elo.c
@@ -89,7 +89,7 @@ static int elo_raw_event(struct hid_device *hdev, struct hid_report *report,
switch (report->id) {
case 0:
- if (data[0] == 'T') { /* Mandatory ELO packet marker */
+ if (size >= 8 && data[0] == 'T') { /* Mandatory ELO packet marker */
elo_process_data(hidinput->input, data, size);
return 1;
}
--
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
* [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 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
* 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
* 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
* [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
* [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 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 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 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 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] 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
* 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
* 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 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 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 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 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] 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 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
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