Re: [PATCH] HID: corsair-void: Check size of status and firmware events before reading them

[sashiko-bot@kernel.org]

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