Re: [PATCH 1/7] hid-asus: Fix up Zenbook Duo report descriptors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The report descriptor fixup for the Zenbook Duo corrupts the descriptor because it incorrectly shifts only 12 bytes and allocates insufficient memory.
--

commit 6124d2bbfc4e5254d102730c30ac761f188e21b0
Author: Joshua Leivenzon <hacker1024@users.sourceforge.net>

hid-asus: Fix up Zenbook Duo report descriptors

This commit adds a quirk for the Zenbook Duo 2024+ keyboards to fix up an
unmapped Asus vendor usage page code. It reuses the existing report
descriptor fixup logic previously used for T100CHI and T90CHI devices.

> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 3f5e96900b67a..ce246efba74d3 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c

[ ... ]

> @@ -1384,17 +1385,20 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  		hid_info(hdev, "Fixing up Asus T100 keyb report descriptor\n");
>  		rdesc[74] &= ~HID_MAIN_ITEM_CONSTANT;
>  	}
> -	/* For the T100CHI/T90CHI keyboard dock */
> -	if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI)) {
> +	/* For the T100CHI/T90CHI keyboard dock and Zenbook Duo 2024+ keyboards */
> +	if (drvdata->quirks & (QUIRK_T100CHI | QUIRK_T90CHI | QUIRK_ZENBOOK_DUO_KEYBOARD)) {
>  		int rsize_orig;
>  		int offs;
>  
>  		if (drvdata->quirks & QUIRK_T100CHI) {
>  			rsize_orig = 403;
>  			offs = 388;
> -		} else {
> +		} else if (drvdata->quirks & QUIRK_T90CHI) {
>  			rsize_orig = 306;
>  			offs = 291;
> +		} else if (drvdata->quirks & QUIRK_ZENBOOK_DUO_KEYBOARD) {
> +			rsize_orig = 257;
> +			offs = 176;
>  		}

Could this introduce a regression by corrupting the Zenbook Duo report
descriptor when failing to shift the remainder of the items correctly?

For the Zenbook Duo, the descriptor is 257 bytes long and the offset is 176,
leaving 79 bytes after the replaced item. However, the shared fixup logic
below this code hardcodes a memmove of exactly 12 bytes:

drivers/hid/hid-asus.c:asus_report_fixup() {
    ...
    memcpy(new_rdesc, rdesc, rsize_orig);
    *rsize = rsize_orig + 1;
    rdesc = new_rdesc;

    memmove(rdesc + offs + 4, rdesc + offs + 2, 12);
    rdesc[offs] = 0x19;
    ...
}

Does shifting only 12 bytes overwrite the valid original bytes at offsets 190
and 191, and leave the remaining 67 bytes unshifted?

Additionally, does the new_rdesc allocation properly account for the shift?

Replacing a 2-byte item with a 4-byte item requires shifting the remaining
items forward by 2 bytes. Allocating only rsize_orig + 1 could lead to a
1-byte out-of-bounds write if all 79 bytes are shifted properly.

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