* Re: [PATCH v5 0/5] HID: asus: security fixes and more hardware support
From: Antheas Kapenekakis @ 2026-06-29 21:13 UTC (permalink / raw)
To: Denis Benato
Cc: Denis Benato, Jiri Kosina, linux-kernel, linux-input,
Benjamin Tissoires, Luke D . Jones, Mateusz Schyboll,
Connor Belli
In-Reply-To: <abca959e-8757-426c-b641-f18cebfbeed5@gmail.com>
On Mon, 29 Jun 2026 at 23:11, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 6/29/26 22:21, Antheas Kapenekakis wrote:
> > On Mon, 29 Jun 2026 at 22:12, Denis Benato <denis.benato@linux.dev> wrote:
> >>
> >> On 6/29/26 11:21, Jiri Kosina wrote:
> >>> On Fri, 19 Jun 2026, Denis Benato wrote:
> >>>
> >>>> Hi all,
> >>>>
> >>>> I have added support for controlling the (way too bright) XG mobile
> >>>> LEDs in hid-asus and added the i2c version of already supported
> >>>> hardware that was probed only when it's a USB: these are two separate
> >>>> features changes and are the only two that are not fixes for
> >>>> pre-existing issue (see below).
> >>>>
> >>>> Auto-review bot has spotted a bunch of pre-existing problems alongside
> >>>> problems in my own code therefore at this point I am going to fix the
> >>>> more problems I can and including those fixes and improvements in this
> >>>> patchset.
> >>>>
> >>>> For v4 I decided to follow Antheas' suggestion of reusing the existing
> >>>> workqueue and by making it more generic I solved a good bunch of issues.
> >>>>
> >>>> The v5 iteration is simply me fixing a bunch of bugs in new code spotted
> >>>> by the bot. Thanks for providing such a useful tool!
> >>>>
> >>>> On a side node this patchset has a few more warnings: specifically
> >>>> "WARNING: Prefer kzalloc_obj over kzalloc with sizeof" but it's a false
> >>>> positive as that would introduce sleeping calls in atomic contexts.
> >>> Denis,
> >>>
> >>> thanks. Could you please flag which patches you'd prefer to go in still
> >>> for 7.1 and which ones are not critical and could wait for 7.2? The whole
> >>> lot is quite big.
> >> Hi Jiri!
> >>
> >> Patch 1 contains multiple fixes, mainly use of sleeping calls in atomic contexts:
> >> I'd say slap it in 7.1 fixes.
> >>
> >> Patch 2 is a minor improvement removing an OOM message to align the driver
> >> to newer code where that would have been a flagged warning: probably a fix for
> >> 7.1 but there's no rush for this one.
> >>
> >> Patch 3 is a logic fix, condition is very rare (not sure if even possible) but belongs
> >> to 7.1 almost certainly.
> > I would like to take my time on reviewing patch 4 in the next cycle.
> > The patch 5 does the backlight for the station? That looked _fine_ to
> > me but a second pair of eyes would be good because it is big.
> Mmhh I think you may have a mistake here? Patch 4 has your reviewed-by
> and it's for the xgm led.
>
> Patch 5 adds backlight for TUFs 2025 and 2026 models: asus is using
> the (almost) same firmware as recent ROGs just i2c instead of usb,
> so patch 5 add i2c to the list.
I want to review for the next cycle:
HID: asus: refactor the two workqueues and init sequence
Antheas
> > Antheas
> >
> >> Patch 4 new feature support: goes to 7.2.
> >>
> >> Patch 5 unsure. It adds support to the backlight, but to properly do it I'll need
> >> to wait for lamparray to be merged since autonomous mode must be disabled
> >> to take effect. I'd say 7.2 just because of the missing lamparray. Merging in 7.1
> >> has no downsides, yet IDK if putting in 7.1 violates some rule (probably yes?).
> >>
> >> Please give a (very) quick read to the following link:
> >> https://github.com/OpenGamingCollective/asusctl/issues/119#issuecomment-4723566635
> >>
> >> This is something I am full of tickets for, probably we have reached the thousand
> >> between discord, github and gitlab: very prominent issue and I'm eagerly waiting
> >> for lamparray support to appear upstream.
> >>
> >> Thanks,
> >> Denis
> >>> Thanks,
> >>>
>
^ permalink raw reply
* Re: [PATCH] HID: hiddev: keep state alive through disconnect unlock
From: Hillf Danton @ 2026-06-29 22:10 UTC (permalink / raw)
To: Yousef Alhouseen
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-usb,
linux-kernel, syzbot+563191a4939ddbfe73d4
In-Reply-To: <CAMuQ4bUuSP=J6Fv=TuzPL6Y34NC3v-rC-O8kGqh86gbUcF8H+g@mail.gmail.com>
[ feel free not to send top reply ]
On Mon, 29 Jun 2026 08:57:02 -0500 Yousef Alhouseen wrote:
> The missing interleaving is that mutex_unlock() clears the owner
> before taking wait_lock. A contender already spinning on existancelock
> can acquire it after that owner clear, reach the final-release path,
> and free hiddev before the disconnecting task reaches
> raw_spin_lock_irqsave(&lock->wait_lock, ...). It does not need to be
> woken by the disconnecting task.
>
In __mutex_unlock_slowpath() the wait_lock is taken because of MUTEX_FLAG_WAITERS
after clearing the lock owner, so it is bug in the mutex backyard instead if
mutex is freed without pending waiters handled.
^ permalink raw reply
* [PATCH] HID: corsair-void: Check size of status and firmware events before reading them
From: Stuart Hayhurst @ 2026-06-30 0:40 UTC (permalink / raw)
To: linux-input
Cc: Stuart Hayhurst, linux-kernel, Benjamin Tissoires, Jiri Kosina,
stable
Malformed status and firmware events could cause an out-of-bounds read since
the size wasn't being checked. Check the size and warn on unexpected values to
avoid this.
Fixes: 6ea2a6fd3872 ("HID: corsair-void: Add Corsair Void headset family driver")
Cc: stable@vger.kernel.org
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
---
drivers/hid/hid-corsair-void.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/hid/hid-corsair-void.c b/drivers/hid/hid-corsair-void.c
index 5e9a5b8f7f16..fdcc4b8cd272 100644
--- a/drivers/hid/hid-corsair-void.c
+++ b/drivers/hid/hid-corsair-void.c
@@ -92,6 +92,9 @@
#define CORSAIR_VOID_STATUS_REPORT_ID 0x64
#define CORSAIR_VOID_FIRMWARE_REPORT_ID 0x66
+#define CORSAIR_VOID_STATUS_REPORT_SIZE 5
+#define CORSAIR_VOID_FIRMWARE_REPORT_SIZE 5
+
#define CORSAIR_VOID_USB_SIDETONE_REQUEST 0x1
#define CORSAIR_VOID_USB_SIDETONE_REQUEST_TYPE 0x21
#define CORSAIR_VOID_USB_SIDETONE_VALUE 0x200
@@ -742,6 +745,11 @@ static int corsair_void_raw_event(struct hid_device *hid_dev,
/* Description of packets are documented at the top of this file */
if (hid_report->id == CORSAIR_VOID_STATUS_REPORT_ID) {
+ if (size != CORSAIR_VOID_STATUS_REPORT_SIZE) {
+ hid_warn(hid_dev, "unexpected status report of size %d", size);
+ return 1;
+ }
+
drvdata->mic_up = FIELD_GET(CORSAIR_VOID_MIC_MASK, data[2]);
drvdata->connected = (data[3] == CORSAIR_VOID_WIRELESS_CONNECTED) ||
drvdata->is_wired;
@@ -750,6 +758,11 @@ static int corsair_void_raw_event(struct hid_device *hid_dev,
FIELD_GET(CORSAIR_VOID_CAPACITY_MASK, data[2]),
data[3], data[4]);
} else if (hid_report->id == CORSAIR_VOID_FIRMWARE_REPORT_ID) {
+ if (size != CORSAIR_VOID_FIRMWARE_REPORT_SIZE) {
+ hid_warn(hid_dev, "unexpected firmware report of size %d", size);
+ return 1;
+ }
+
drvdata->fw_receiver_major = data[1];
drvdata->fw_receiver_minor = data[2];
drvdata->fw_headset_major = data[3];
--
2.53.0
^ permalink raw reply related
* [PATCH] HID: corsair-void: Fix firmware event packet description
From: Stuart Hayhurst @ 2026-06-30 0:43 UTC (permalink / raw)
To: linux-input
Cc: Stuart Hayhurst, linux-kernel, Benjamin Tissoires, Jiri Kosina
The size was incorrectly stated as 4 bytes since the ID was missed out.
Add the ID in and correct the indices for the firmware versions.
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
---
drivers/hid/hid-corsair-void.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-corsair-void.c b/drivers/hid/hid-corsair-void.c
index 5e9a5b8f7f16..ce92e3a9c058 100644
--- a/drivers/hid/hid-corsair-void.c
+++ b/drivers/hid/hid-corsair-void.c
@@ -51,20 +51,23 @@
/* Receiver report information: (ID 102) */
/* -------------------------------------------------------------------------- */
/*
- * When queried, the recevier responds with 4 bytes to describe the firmware
- * The first 2 bytes are for the receiver, the second 2 are the headset
+ * When queried, the receiver responds with 5 bytes to describe the firmware
+ * The first byte is the ID, then 2 bytes for the receiver, then 2 for the headset
* The headset firmware version will be 0 if no headset is connected
*
- * 0: Recevier firmware major version
+ * 0: Report ID
+ * 102 for the firmware packet
+ *
+ * 1: Receiver firmware major version
* Major version of the receiver's firmware
*
- * 1: Recevier firmware minor version
+ * 2: Receiver firmware minor version
* Minor version of the receiver's firmware
*
- * 2: Headset firmware major version
+ * 3: Headset firmware major version
* Major version of the headset's firmware
*
- * 3: Headset firmware minor version
+ * 4: Headset firmware minor version
* Minor version of the headset's firmware
*/
/* -------------------------------------------------------------------------- */
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] HID: corsair-void: Check size of status and firmware events before reading them
From: sashiko-bot @ 2026-06-30 0:53 UTC (permalink / raw)
To: Stuart Hayhurst; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260630004003.579171-2-stuart.a.hayhurst@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Un-ratelimited kernel warning in a high-frequency packet processing path.
- [Medium] Overly strict bounds checking on report sizes incorrectly rejects valid, padded reports.
--
commit 867047370d14d44d5c13655e52c433ab8be9dd4a
Author: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
HID: corsair-void: Check size of status and firmware events before reading them
This commit adds bounds checking for status and firmware events in the
corsair-void HID driver to prevent out-of-bounds reads. If the size of the
event does not match the expected size, it logs a warning and returns early.
Cc: stable@vger.kernel.org
> diff --git a/drivers/hid/hid-corsair-void.c b/drivers/hid/hid-corsair-void.c
> index 5e9a5b8f7f167..fdcc4b8cd2720 100644
> --- a/drivers/hid/hid-corsair-void.c
> +++ b/drivers/hid/hid-corsair-void.c
[ ... ]
> @@ -742,6 +745,11 @@ static int corsair_void_raw_event(struct hid_device *hid_dev,
>
> /* Description of packets are documented at the top of this file */
> if (hid_report->id == CORSAIR_VOID_STATUS_REPORT_ID) {
> + if (size != CORSAIR_VOID_STATUS_REPORT_SIZE) {
[Severity: Medium]
Does this overly strict bounds check reject valid, padded reports?
It is common for USB host controllers to pad reports for 32-bit alignment,
or for newer headset firmwares to append data. Checking for an exact match
instead of a minimum length could cause the driver to silently drop these
packets and break headset functionality like battery status.
Would it be safer to check for a minimum length instead?
> + hid_warn(hid_dev, "unexpected status report of size %d", size);
[Severity: High]
Can a malfunctioning or malicious USB device flood the kernel log here?
Since this warning is un-ratelimited and occurs in the high-frequency packet
processing path of corsair_void_raw_event(), a device continuously sending
incorrectly sized reports could spam dmesg and potentially stall the CPU on
a slow serial console.
Should this use a ratelimited print, such as hid_warn_ratelimited() or
dev_warn_ratelimited()?
> + return 1;
> + }
> +
> drvdata->mic_up = FIELD_GET(CORSAIR_VOID_MIC_MASK, data[2]);
> drvdata->connected = (data[3] == CORSAIR_VOID_WIRELESS_CONNECTED) ||
> drvdata->is_wired;
> @@ -750,6 +758,11 @@ static int corsair_void_raw_event(struct hid_device *hid_dev,
> FIELD_GET(CORSAIR_VOID_CAPACITY_MASK, data[2]),
> data[3], data[4]);
> } else if (hid_report->id == CORSAIR_VOID_FIRMWARE_REPORT_ID) {
> + if (size != CORSAIR_VOID_FIRMWARE_REPORT_SIZE) {
[Severity: Medium]
This has the same strict bounds checking issue as the status report check
above. Does this also need to be converted to a minimum length check?
> + hid_warn(hid_dev, "unexpected firmware report of size %d", size);
[Severity: High]
This warning is also un-ratelimited. Can this be changed to use a
ratelimited print to prevent log flooding from malformed firmware reports?
> + return 1;
> + }
> +
> drvdata->fw_receiver_major = data[1];
> drvdata->fw_receiver_minor = data[2];
> drvdata->fw_headset_major = data[3];
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260630004003.579171-2-stuart.a.hayhurst@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH] HID: corsair-void: Fix firmware event packet description
From: sashiko-bot @ 2026-06-30 0:54 UTC (permalink / raw)
To: Stuart Hayhurst; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260630004323.589696-3-stuart.a.hayhurst@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The `corsair_void_raw_event()` function lacks a bounds check on the `size` of the incoming HID report, which can lead to an out-of-bounds read when accessing elements up to `data[4]`.
--
commit d2f977ace02628f486f8135ff442479be032fad6
Author: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
HID: corsair-void: Fix firmware event packet description
This commit updates the comment describing the firmware event packet in the
corsair-void HID driver. It corrects the stated size to 5 bytes to include
the ID, and updates the corresponding indices for the firmware versions.
> diff --git a/drivers/hid/hid-corsair-void.c b/drivers/hid/hid-corsair-void.c
> index 5e9a5b8f7f167..ce92e3a9c0580 100644
> --- a/drivers/hid/hid-corsair-void.c
> +++ b/drivers/hid/hid-corsair-void.c
> @@ -51,20 +51,23 @@
> /* Receiver report information: (ID 102) */
> /* -------------------------------------------------------------------------- */
> /*
> - * When queried, the recevier responds with 4 bytes to describe the firmware
> - * The first 2 bytes are for the receiver, the second 2 are the headset
> + * When queried, the receiver responds with 5 bytes to describe the firmware
> + * The first byte is the ID, then 2 bytes for the receiver, then 2 for the headset
> * The headset firmware version will be 0 if no headset is connected
> *
> - * 0: Recevier firmware major version
> + * 0: Report ID
> + * 102 for the firmware packet
> + *
> + * 1: Receiver firmware major version
> * Major version of the receiver's firmware
> *
> - * 1: Recevier firmware minor version
> + * 2: Receiver firmware minor version
> * Minor version of the receiver's firmware
> *
> - * 2: Headset firmware major version
> + * 3: Headset firmware major version
> * Major version of the headset's firmware
> *
> - * 3: Headset firmware minor version
> + * 4: Headset firmware minor version
> * Minor version of the headset's firmware
> */
[Severity: High]
This is a pre-existing issue, but does corsair_void_raw_event() lack a bounds
check on the size of the incoming HID report?
When the device sends a report matching the firmware report ID, it appears
the handler unconditionally accesses elements up to data[4]:
drivers/hid/hid-corsair-void.c:corsair_void_raw_event() {
...
drvdata->fw_receiver_major = data[1];
drvdata->fw_receiver_minor = data[2];
drvdata->fw_headset_major = data[3];
drvdata->fw_headset_minor = data[4];
...
}
If a malicious USB device sends a crafted HID descriptor defining a small
maximum report size, and then sends a 1-byte firmware report, can this lead
to an out-of-bounds read past the end of the report buffer?
It looks like the same issue might exist for the status report, which also
reads up to data[4] unconditionally:
drivers/hid/hid-corsair-void.c:corsair_void_raw_event() {
...
drvdata->mic_up = FIELD_GET(CORSAIR_VOID_MIC_MASK, data[2]);
drvdata->connected = (data[3] == CORSAIR_VOID_WIRELESS_CONNECTED) ||
drvdata->is_wired;
corsair_void_process_receiver(drvdata,
FIELD_GET(CORSAIR_VOID_CAPACITY_MASK, data[2]),
data[3], data[4]);
...
}
Could these out-of-bounds bytes be subsequently exposed to userspace via
sysfs attributes, resulting in a kernel memory information leak?
Could this also potentially trigger a kernel panic if the dynamically
allocated report buffer is small?
> /* -------------------------------------------------------------------------- */
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260630004323.589696-3-stuart.a.hayhurst@gmail.com?part=1
^ permalink raw reply
* [PATCH v2] HID: corsair-void: Check size of status and firmware events before reading them
From: Stuart Hayhurst @ 2026-06-30 1:06 UTC (permalink / raw)
To: linux-input
Cc: Stuart Hayhurst, linux-kernel, Benjamin Tissoires, Jiri Kosina,
stable
Malformed status and firmware events could cause an out-of-bounds read since
the size wasn't being checked. Check the size and warn on unexpected values to
avoid this.
Fixes: 6ea2a6fd3872 ("HID: corsair-void: Add Corsair Void headset family driver")
Cc: stable@vger.kernel.org
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
---
v1 -> v2:
- Ratelimit the warnings
- Accept packets larger than the expected size
---
drivers/hid/hid-corsair-void.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/hid/hid-corsair-void.c b/drivers/hid/hid-corsair-void.c
index 5e9a5b8f7f16..071a663a6c26 100644
--- a/drivers/hid/hid-corsair-void.c
+++ b/drivers/hid/hid-corsair-void.c
@@ -92,6 +92,9 @@
#define CORSAIR_VOID_STATUS_REPORT_ID 0x64
#define CORSAIR_VOID_FIRMWARE_REPORT_ID 0x66
+#define CORSAIR_VOID_STATUS_REPORT_SIZE 5
+#define CORSAIR_VOID_FIRMWARE_REPORT_SIZE 5
+
#define CORSAIR_VOID_USB_SIDETONE_REQUEST 0x1
#define CORSAIR_VOID_USB_SIDETONE_REQUEST_TYPE 0x21
#define CORSAIR_VOID_USB_SIDETONE_VALUE 0x200
@@ -742,6 +745,13 @@ static int corsair_void_raw_event(struct hid_device *hid_dev,
/* Description of packets are documented at the top of this file */
if (hid_report->id == CORSAIR_VOID_STATUS_REPORT_ID) {
+ if (size < CORSAIR_VOID_STATUS_REPORT_SIZE) {
+ hid_warn_ratelimited(hid_dev,
+ "unexpected status report of size %d",
+ size);
+ return 1;
+ }
+
drvdata->mic_up = FIELD_GET(CORSAIR_VOID_MIC_MASK, data[2]);
drvdata->connected = (data[3] == CORSAIR_VOID_WIRELESS_CONNECTED) ||
drvdata->is_wired;
@@ -750,6 +760,13 @@ static int corsair_void_raw_event(struct hid_device *hid_dev,
FIELD_GET(CORSAIR_VOID_CAPACITY_MASK, data[2]),
data[3], data[4]);
} else if (hid_report->id == CORSAIR_VOID_FIRMWARE_REPORT_ID) {
+ if (size < CORSAIR_VOID_FIRMWARE_REPORT_SIZE) {
+ hid_warn_ratelimited(hid_dev,
+ "unexpected firmware report of size %d",
+ size);
+ return 1;
+ }
+
drvdata->fw_receiver_major = data[1];
drvdata->fw_receiver_minor = data[2];
drvdata->fw_headset_major = data[3];
--
2.53.0
^ permalink raw reply related
page: | 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