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

[Mario Limonciello <mario.limonciello@amd.com>]

On 9/10/26 12:17, sashiko-bot@kernel.org wrote:
> 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);
> [ ... ]
> 

Curtis - can you check how the headset handles those cases?