Re: [PATCH v11 12/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(s) to consider:
- [High] HID: asus: slab out-of-bounds read in asus_kbd_get_functions()
--

commit c08fc8186d8cfc7d33787c731647d924103580c9
Author: Denis Benato <benato.denis96@gmail.com>

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

This commit derives feature report transfer lengths dynamically from the
device's HID report descriptor instead of using hardcoded sizes. It adds
asus_report_len() and asus_kbd_get_report(), updates asus_kbd_set_report() to
zero-pad short payloads to the report length, and converts callers to size
their buffers per report.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index ea27495c3df66..fe2a326ccd861 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c

[ ... ]

> @@ -5426,7 +5521,7 @@ static int asus_kbd_get_functions(struct hid_device *hdev,
>  				  u8 report_id)
>  {
>  	const u8 buf[] = { report_id, 0x05, 0x20, 0x31, 0x00, 0x08 };
> -	u8 *readbuf;
> +	size_t hw_len = asus_report_len(hdev, report_id);
>  	int ret;
>  
>  	ret = asus_kbd_set_report(hdev, buf, sizeof(buf));
> @@ -5435,23 +5530,22 @@ static int asus_kbd_get_functions(struct hid_device *hdev,
>  		return ret;
>  	}
>  
> -	readbuf = kzalloc(FEATURE_KBD_REPORT_SIZE, GFP_KERNEL);
> +	if (!hw_len)
> +		return -EINVAL;
> +
> +	u8 *readbuf __free(kfree) = kzalloc(hw_len, GFP_KERNEL);
>  	if (!readbuf)
>  		return -ENOMEM;
>  
> -	ret = hid_hw_raw_request(hdev, report_id, readbuf,
> -				 FEATURE_KBD_REPORT_SIZE, HID_FEATURE_REPORT,
> -				 HID_REQ_GET_REPORT);
> +	ret = asus_kbd_get_report(hdev, report_id, readbuf, hw_len);
>  	if (ret < 0) {
>  		hid_err(hdev, "Asus failed to request functions: %d\n", ret);
> -		kfree(readbuf);
>  		return ret;
>  	}
>  
>  	*kbd_func = readbuf[6];

[Severity: High]
Can this read past the end of readbuf[] when a device descriptor declares a
6-byte feature report for report_id?

Previously, asus_kbd_get_functions() in drivers/hid/hid-asus.c allocated
readbuf with FEATURE_KBD_REPORT_SIZE (64 bytes). With this change, readbuf is
allocated with size hw_len.

In asus_kbd_get_functions(), sizeof(buf) is 6, so asus_kbd_set_report()
accepts hw_len == 6 via:

    if (!hw_len || buf_size > hw_len)
        return -EINVAL;

asus_kbd_get_functions() then only checks !hw_len before allocating readbuf of
6 bytes and reading readbuf[6] (the 7th byte), resulting in a 1-byte slab
out-of-bounds read during probe via
asus_probe()->asus_kbd_register_leds()->asus_kbd_get_functions().

Should asus_kbd_get_functions() check that hw_len is at least 7 (or
hw_len <= 6) before allocating readbuf and indexing readbuf[6]?

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