Re: [PATCH] HID: usbhid: add HID_QUIRK_SKIP_FEATURE_REPORT_ID

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] HID: missing HID_QUIRK_SKIP_FEATURE_REPORT_ID check in usbhid_get_raw_report
--

commit b5d7d6ff27fe5a08bcc6f67c8ed596cb786d6404
Author: Fedor Gubenkov <writesfaster@gmail.com>

HID: usbhid: add HID_QUIRK_SKIP_FEATURE_REPORT_ID

Some HID dongles implement a stateful challenge/response over HID
feature reports. This patch introduces a quirk to omit the report ID from
the SET_REPORT payload, fixing a stall when interacting with the USBKey dongle.

> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -914,8 +914,10 @@ static int usbhid_set_raw_report(struct hid_device *hid, unsigned int reportnum,
>  	int ret, skipped_report_id = 0;
>  
>  	/* Byte 0 is the report number. Report data starts at byte 1.*/
> -	if ((rtype == HID_OUTPUT_REPORT) &&
> -	    (hid->quirks & HID_QUIRK_SKIP_OUTPUT_REPORT_ID))
> +	if (((rtype == HID_OUTPUT_REPORT) &&
> +	     (hid->quirks & HID_QUIRK_SKIP_OUTPUT_REPORT_ID)) ||
> +	    ((rtype == HID_FEATURE_REPORT) &&
> +	     (hid->quirks & HID_QUIRK_SKIP_FEATURE_REPORT_ID)))
>  		buf[0] = 0;
>  	else
>  		buf[0] = reportnum;

[Severity: Medium]
Since this quirk is used for a challenge/response over feature reports,
does usbhid_get_raw_report() also need to be updated to handle the response?

If the device omits the report ID in its response, it appears
usbhid_get_raw_report() will not offset the buffer for numbered reports:

drivers/hid/usbhid/hid-core.c:usbhid_get_raw_report() {
	/* Byte 0 is the report number. Report data starts at byte 1.*/
	buf[0] = report_number;
	if (report_number == 0x0) {
		/* Offset the return buffer by 1, so that the report ID
		   will remain in byte 0. */
		buf++;
		count--;
		skipped_report_id = 1;
	}
...
}

Will this cause usb_control_msg() to overwrite buf[0] (the report ID) with the
first byte of the device's raw payload, resulting in a misaligned payload for
user-space applications reading these feature reports?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015102601.1235724-1-writesfaster@gmail.com?part=1