Re: [PATCH v2] HID: multitouch: fix slab out-of-bounds access in mt_report_fixup()

[Jiri Slaby <jirislaby@kernel.org>]

On 23. 07. 25, 13:00, Qasim Ijaz wrote:
> A malicious HID device can trigger a slab out-of-bounds during
> mt_report_fixup() by passing in report descriptor smaller than
> 607 bytes. mt_report_fixup() attempts to patch byte offset 607
> of the descriptor with 0x25 by first checking if byte offset
> 607 is 0x15 however it lacks bounds checks to verify if the
> descriptor is big enough before conducting this check. Fix
> this bug by ensuring the descriptor size is at least 608
> bytes before accessing it.
> 
> Below is the KASAN splat after the out of bounds access happens:
> 
> [   13.671954] ==================================================================
> [   13.672667] BUG: KASAN: slab-out-of-bounds in mt_report_fixup+0x103/0x110
...
> [...]
> 
> Fixes: c8000deb6836 ("HID: multitouch: Add support for GT7868Q")
> Cc: stable@vger.kernel.org
> Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>

LGTM
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>

> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -1503,6 +1503,14 @@ static const __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>   	if (hdev->vendor == I2C_VENDOR_ID_GOODIX &&
>   	    (hdev->product == I2C_DEVICE_ID_GOODIX_01E8 ||
>   	     hdev->product == I2C_DEVICE_ID_GOODIX_01E9)) {
> +		if (*size < 608) {
> +			dev_info(

Except I would not add \n to the line above.

> +				&hdev->dev,
> +				"GT7868Q fixup: report descriptor is only %u bytes, skipping\n",
> +				*size);
> +			return rdesc;
> +		}
> +
>   		if (rdesc[607] == 0x15) {
>   			rdesc[607] = 0x25;
>   			dev_info(

thanks,
-- 
js
suse labs