Re: [PATCH 2/3] HID: valve-index: Reboot headset on system power transitions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incorrect buffer formatting and report ID targeting in `hid_hw_raw_request` fallback for an unnumbered HID report.
--

commit 4526fbce2701e5a40089fba6ae186ded76bc0864
Author: Mario Limonciello <mario.limonciello@amd.com>
Subject: HID: valve-index: Reboot headset on system power transitions

This commit introduces a new HID driver for the Valve Index headset to
reboot it on system power transitions. It sends a specific 64-byte report
to recover the EDID service after a DisplayPort link shutdown.

Link: https://github.com/ValveSoftware/SteamVR-for-Linux/issues/939

> diff --git a/drivers/hid/hid-valve-index.c b/drivers/hid/hid-valve-index.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/hid/hid-valve-index.c
[ ... ]
> +static void valve_index_reboot(struct hid_device *hdev, bool wake)
> +{
> +	u8 *report;
> +	int ret;
> +
> +	if (!valve_index_has_reboot_report(hdev))
> +		return;
> +
> +	/* USB transfer buffers must be DMA-able, so not on the stack. */
> +	report = kzalloc(VALVE_INDEX_REPORT_SIZE, GFP_KERNEL);
> +	if (!report)
> +		return;
> +	report[0] = VALVE_INDEX_REBOOT_REPORT_ID;

[Severity: Medium]
Is the report buffer sized correctly for an unnumbered report fallback?

The HID core and usbhid transport expect unnumbered report buffers to be
prefixed with a dummy 0x00 byte (so payload size + 1) for control transfers.
Allocating exactly 64 bytes and placing the payload command at index 0 might
break control transfer formatting in usbhid_set_raw_report if the fallback
path below is taken.

[ ... ]
> +	/* Use the same interrupt-out then SET_REPORT fallback as hidraw. */
> +	ret = hid_hw_output_report(hdev, report, VALVE_INDEX_REPORT_SIZE);
> +	if (ret == -ENOSYS)
> +		ret = hid_hw_raw_request(hdev, report[0], report,
> +					 VALVE_INDEX_REPORT_SIZE,
> +					 HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);

[Severity: Medium]
Does this properly target the unnumbered report during the fallback?

By passing report[0] (which contains 0x16) as the reportnum argument, the
fallback control transfer will target Output Report ID 0x16 instead of the
required unnumbered report (ID 0). If hid_hw_output_report returns -ENOSYS
and this fallback is taken, the headset might reject the transfer, causing
the reboot command to fail.

> +	if (ret < 0)
> +		hid_warn(hdev, "failed to reboot headset: %d\n", ret);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910170254.833871-1-mario.limonciello@amd.com?part=2