Re: [PATCH v14 01/18] HID: asus: size feature reports from the HID report descriptor

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] HID: asus: report type mismatch in asus_has_report_id() and
  asus_report_len()
--

commit a49f4ba7e2479238fb06ec5527d0ef52ccd84b83
Author: Denis Benato <denis.benato@linux.dev>

HID: asus: size feature reports from the HID report descriptor

This commit derives feature report transfer lengths from the device's HID
report descriptor via asus_report_len() instead of using hardcoded report
sizes.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index f6aa1be09ad53..46643022b68d0 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
[ ... ]
> +static size_t asus_report_len(struct hid_device *hdev, u8 report_id)
> +{
> +	struct hid_report *report;
> +
> +	report = asus_find_report(hdev, HID_FEATURE_REPORT, report_id);
> +
> +	return report ? hid_report_len(report) : 0;
> +}
[ ... ]
> @@ -604,8 +687,13 @@ static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
>  	 */
>  	const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
>  		     0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
> +	size_t hw_len = asus_report_len(hdev, report_id);
> +	size_t cmp_len;
>  	int ret;
>  
> +	if (!hw_len)
> +		return -EINVAL;
[ ... ]
> @@ -958,17 +1054,12 @@ static void validate_mcu_fw_version(struct hid_device *hdev, int idProduct)
>  
>  static bool asus_has_report_id(struct hid_device *hdev, u16 report_id)
>  {
> -	struct hid_report *report;
> -	int t;
> +	if (report_id >= HID_MAX_IDS)
> +		return false;
>  
> -	for (t = HID_INPUT_REPORT; t <= HID_FEATURE_REPORT; t++) {
> -		list_for_each_entry(report, &hdev->report_enum[t].report_list, list) {
> -			if (report->id == report_id)
> -				return true;
> -		}
> -	}
> -
> -	return false;
> +	return asus_find_report(hdev, HID_INPUT_REPORT, report_id) ||
> +	       asus_find_report(hdev, HID_OUTPUT_REPORT, report_id) ||
> +	       asus_find_report(hdev, HID_FEATURE_REPORT, report_id);
>  }

[Severity: Medium]
Should asus_has_report_id() and asus_report_len() check the same report
types?

asus_has_report_id() matches HID_INPUT_REPORT, HID_OUTPUT_REPORT, and
HID_FEATURE_REPORT, and its callers gate feature-report operations such as
asus_initialize_reports() -> asus_kbd_init():

drivers/hid/hid-asus.c:asus_initialize_reports() {
	for (int r = 0; r < ARRAY_SIZE(asus_report_id_init); r++) {
		if (asus_has_report_id(hdev, asus_report_id_init[r])) {
			ret = asus_kbd_init(hdev, asus_report_id_init[r]);
			if (ret < 0)
				hid_warn(hdev, "Failed to initialize 0x%x: %d.\n",
					 asus_report_id_init[r], ret);
		}
	}
}

However, asus_report_len() only checks HID_FEATURE_REPORT and returns 0
otherwise.  On devices whose descriptor declares report 0x5a only as an
input report (such as keyboards using QUIRK_G752_KEYBOARD, where the fixed
descriptor in asus_report_fixup() only defines an Input item for report
0x5a), asus_has_report_id() returns true while asus_kbd_init() now fails
with -EINVAL and logs a warning on probe and reset_resume.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106142334.1881-1-denis.benato@linux.dev?part=1