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 vulnerability by ensuring the descriptor size is greater than or equal to 608 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 [ 13.673297] Read of size 1 at addr ffff888103df39df by task kworker/0:1/10 [ 13.673297] [ 13.673297] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.15.0-00005-gec5d573d83f4-dirty #3 [ 13.673297] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/04 [ 13.673297] Call Trace: [ 13.673297] <TASK> [ 13.673297] dump_stack_lvl+0x5f/0x80 [ 13.673297] print_report+0xd1/0x660 [ 13.673297] kasan_report+0xe5/0x120 [ 13.673297] __asan_report_load1_noabort+0x18/0x20 [ 13.673297] mt_report_fixup+0x103/0x110 [ 13.673297] hid_open_report+0x1ef/0x810 [ 13.673297] mt_probe+0x422/0x960 [ 13.673297] hid_device_probe+0x2e2/0x6f0 [ 13.673297] really_probe+0x1c6/0x6b0 [ 13.673297] __driver_probe_device+0x24f/0x310 [ 13.673297] driver_probe_device+0x4e/0x220 [ 13.673297] __device_attach_driver+0x169/0x320 [ 13.673297] bus_for_each_drv+0x11d/0x1b0 [ 13.673297] __device_attach+0x1b8/0x3e0 [ 13.673297] device_initial_probe+0x12/0x20 [ 13.673297] bus_probe_device+0x13d/0x180 [ 13.673297] device_add+0xe3a/0x1670 [ 13.673297] hid_add_device+0x31d/0xa40 [...]
Fixes: c8000deb6836 ("HID: multitouch: Add support for GT7868Q") Cc: stable@vger.kernel.org Signed-off-by: Qasim Ijaz qasdev00@gmail.com Reviewed-by: Dmitry Savin envelsavinds@gmail.com --- drivers/hid/hid-multitouch.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 7ac8e16e6158..af4abe3ba410 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -1461,18 +1461,25 @@ 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 (rdesc[607] == 0x15) { - rdesc[607] = 0x25; - dev_info( - &hdev->dev, - "GT7868Q report descriptor fixup is applied.\n"); + if (*size >= 608) { + if (rdesc[607] == 0x15) { + rdesc[607] = 0x25; + dev_info( + &hdev->dev, + "GT7868Q report descriptor fixup is applied.\n"); + } else { + dev_info( + &hdev->dev, + "The byte is not expected for fixing the report descriptor. \ + It's possible that the touchpad firmware is not suitable for applying the fix. \ + got: %x\n", + rdesc[607]); + } } else { dev_info( &hdev->dev, - "The byte is not expected for fixing the report descriptor. \ -It's possible that the touchpad firmware is not suitable for applying the fix. \ -got: %x\n", - rdesc[607]); + "GT7868Q fixup: report descriptor only %u bytes, skipping\n", + *size); } }
Reviewed-by: Dmitry Savin envelsavinds@gmail.com
On Tue, 22 Jul 2025 at 09:00, Qasim Ijaz qasdev00@gmail.com 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 vulnerability by ensuring the descriptor size is greater than or equal to 608 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 [ 13.673297] Read of size 1 at addr ffff888103df39df by task kworker/0:1/10 [ 13.673297] [ 13.673297] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.15.0-00005-gec5d573d83f4-dirty #3 [ 13.673297] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/04 [ 13.673297] Call Trace: [ 13.673297] <TASK> [ 13.673297] dump_stack_lvl+0x5f/0x80 [ 13.673297] print_report+0xd1/0x660 [ 13.673297] kasan_report+0xe5/0x120 [ 13.673297] __asan_report_load1_noabort+0x18/0x20 [ 13.673297] mt_report_fixup+0x103/0x110 [ 13.673297] hid_open_report+0x1ef/0x810 [ 13.673297] mt_probe+0x422/0x960 [ 13.673297] hid_device_probe+0x2e2/0x6f0 [ 13.673297] really_probe+0x1c6/0x6b0 [ 13.673297] __driver_probe_device+0x24f/0x310 [ 13.673297] driver_probe_device+0x4e/0x220 [ 13.673297] __device_attach_driver+0x169/0x320 [ 13.673297] bus_for_each_drv+0x11d/0x1b0 [ 13.673297] __device_attach+0x1b8/0x3e0 [ 13.673297] device_initial_probe+0x12/0x20 [ 13.673297] bus_probe_device+0x13d/0x180 [ 13.673297] device_add+0xe3a/0x1670 [ 13.673297] hid_add_device+0x31d/0xa40 [...]
Fixes: c8000deb6836 ("HID: multitouch: Add support for GT7868Q") Cc: stable@vger.kernel.org Signed-off-by: Qasim Ijaz qasdev00@gmail.com Reviewed-by: Dmitry Savin envelsavinds@gmail.com
drivers/hid/hid-multitouch.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 7ac8e16e6158..af4abe3ba410 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -1461,18 +1461,25 @@ 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 (rdesc[607] == 0x15) {rdesc[607] = 0x25;dev_info(&hdev->dev,"GT7868Q report descriptor fixup is applied.\n");
if (*size >= 608) {if (rdesc[607] == 0x15) {rdesc[607] = 0x25;dev_info(&hdev->dev,"GT7868Q report descriptor fixup is applied.\n");} else {dev_info(&hdev->dev,"The byte is not expected for fixing the report descriptor. \It's possible that the touchpad firmware is not suitable for applying the fix. \got: %x\n",rdesc[607]);} } else { dev_info( &hdev->dev,
"The byte is not expected for fixing the report descriptor. \-It's possible that the touchpad firmware is not suitable for applying the fix. \ -got: %x\n",
rdesc[607]);
"GT7868Q fixup: report descriptor only %u bytes, skipping\n",*size); } }-- 2.39.5
On 22. 07. 25, 10: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 vulnerability by ensuring the descriptor size is greater than or equal to 608 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 [ 13.673297] Read of size 1 at addr ffff888103df39df by task kworker/0:1/10 [ 13.673297] [ 13.673297] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.15.0-00005-gec5d573d83f4-dirty #3 [ 13.673297] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/04 [ 13.673297] Call Trace: [ 13.673297] <TASK> [ 13.673297] dump_stack_lvl+0x5f/0x80 [ 13.673297] print_report+0xd1/0x660 [ 13.673297] kasan_report+0xe5/0x120 [ 13.673297] __asan_report_load1_noabort+0x18/0x20 [ 13.673297] mt_report_fixup+0x103/0x110 [ 13.673297] hid_open_report+0x1ef/0x810 [ 13.673297] mt_probe+0x422/0x960 [ 13.673297] hid_device_probe+0x2e2/0x6f0 [ 13.673297] really_probe+0x1c6/0x6b0 [ 13.673297] __driver_probe_device+0x24f/0x310 [ 13.673297] driver_probe_device+0x4e/0x220 [ 13.673297] __device_attach_driver+0x169/0x320 [ 13.673297] bus_for_each_drv+0x11d/0x1b0 [ 13.673297] __device_attach+0x1b8/0x3e0 [ 13.673297] device_initial_probe+0x12/0x20 [ 13.673297] bus_probe_device+0x13d/0x180 [ 13.673297] device_add+0xe3a/0x1670 [ 13.673297] hid_add_device+0x31d/0xa40 [...]
Fixes: c8000deb6836 ("HID: multitouch: Add support for GT7868Q") Cc: stable@vger.kernel.org Signed-off-by: Qasim Ijaz qasdev00@gmail.com Reviewed-by: Dmitry Savin envelsavinds@gmail.com
drivers/hid/hid-multitouch.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 7ac8e16e6158..af4abe3ba410 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -1461,18 +1461,25 @@ 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 (rdesc[607] == 0x15) {rdesc[607] = 0x25;dev_info(&hdev->dev,"GT7868Q report descriptor fixup is applied.\n");
if (*size >= 608) {if (rdesc[607] == 0x15) {rdesc[607] = 0x25;dev_info(&hdev->dev,"GT7868Q report descriptor fixup is applied.\n");} else {dev_info(&hdev->dev,"The byte is not expected for fixing the report descriptor. \It's possible that the touchpad firmware is not suitable for applying the fix. \got: %x\n",
This is wrong. You have all the spaces/tabs in the string now. Drop all the backslashes, and open and close the string on every line.
rdesc[607]);}
As this is superlong and superindented, perhaps introduce a new function for these devices?
} else { dev_info( &hdev->dev,
"The byte is not expected for fixing the report descriptor. \-It's possible that the touchpad firmware is not suitable for applying the fix. \ -got: %x\n",
This was horrid too, yeah.
rdesc[607]);
"GT7868Q fixup: report descriptor only %u bytes, skipping\n",
A predicate missing. Eg. "has only", or "is only".
*size);
thanks,
On Tue, Jul 22, 2025 at 11:16:15AM +0200, Jiri Slaby wrote:
On 22. 07. 25, 10: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 vulnerability by ensuring the descriptor size is greater than or equal to 608 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 [ 13.673297] Read of size 1 at addr ffff888103df39df by task kworker/0:1/10 [ 13.673297] [ 13.673297] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.15.0-00005-gec5d573d83f4-dirty #3 [ 13.673297] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/04 [ 13.673297] Call Trace: [ 13.673297] <TASK> [ 13.673297] dump_stack_lvl+0x5f/0x80 [ 13.673297] print_report+0xd1/0x660 [ 13.673297] kasan_report+0xe5/0x120 [ 13.673297] __asan_report_load1_noabort+0x18/0x20 [ 13.673297] mt_report_fixup+0x103/0x110 [ 13.673297] hid_open_report+0x1ef/0x810 [ 13.673297] mt_probe+0x422/0x960 [ 13.673297] hid_device_probe+0x2e2/0x6f0 [ 13.673297] really_probe+0x1c6/0x6b0 [ 13.673297] __driver_probe_device+0x24f/0x310 [ 13.673297] driver_probe_device+0x4e/0x220 [ 13.673297] __device_attach_driver+0x169/0x320 [ 13.673297] bus_for_each_drv+0x11d/0x1b0 [ 13.673297] __device_attach+0x1b8/0x3e0 [ 13.673297] device_initial_probe+0x12/0x20 [ 13.673297] bus_probe_device+0x13d/0x180 [ 13.673297] device_add+0xe3a/0x1670 [ 13.673297] hid_add_device+0x31d/0xa40 [...]
Fixes: c8000deb6836 ("HID: multitouch: Add support for GT7868Q") Cc: stable@vger.kernel.org Signed-off-by: Qasim Ijaz qasdev00@gmail.com Reviewed-by: Dmitry Savin envelsavinds@gmail.com
drivers/hid/hid-multitouch.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 7ac8e16e6158..af4abe3ba410 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -1461,18 +1461,25 @@ 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 (rdesc[607] == 0x15) {rdesc[607] = 0x25;dev_info(&hdev->dev,"GT7868Q report descriptor fixup is applied.\n");
if (*size >= 608) {if (rdesc[607] == 0x15) {rdesc[607] = 0x25;dev_info(&hdev->dev,"GT7868Q report descriptor fixup is applied.\n");} else {dev_info(&hdev->dev,"The byte is not expected for fixing the report descriptor. \It's possible that the touchpad firmware is not suitable for applying the fix. \got: %x\n",This is wrong. You have all the spaces/tabs in the string now. Drop all the backslashes, and open and close the string on every line.
rdesc[607]);}As this is superlong and superindented, perhaps introduce a new function for these devices?
} else { dev_info( &hdev->dev,
"The byte is not expected for fixing the report descriptor. \-It's possible that the touchpad firmware is not suitable for applying the fix. \ -got: %x\n",
This was horrid too, yeah.
rdesc[607]);
"GT7868Q fixup: report descriptor only %u bytes, skipping\n",A predicate missing. Eg. "has only", or "is only".
Thanks for the feedback Jiri, I took the advice on board, is something like this better?
static const __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *size) { 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( &hdev->dev, "GT7868Q fixup: report descriptor is only %u bytes, skipping\n", *size); return rdesc; }
if (rdesc[607] == 0x15) { rdesc[607] = 0x25; dev_info( &hdev->dev, "GT7868Q fixup: report descriptor fixup is applied.\n"); } else { dev_info(&hdev->dev, "GT7868Q fixup: offset 607 is %x (expected 0x15), " "descriptor may be malformed, skipping\n", rdesc[607]); } }
return rdesc; }
the key changes I made are:
- Move size check to the top, this way the indentation level is decent - get rid of message backslashes - shorten the fixup failure message when rdesc[607] is not 0x15 and make it a bit clearer since this message was the longest - just a minor cleanup - added "is only %u bytes" as you suggested
if this is all good I can send v2.
Thanks qasim
*size);thanks,
js suse labs
On 22. 07. 25, 13:19, Qasim Ijaz wrote:
On Tue, Jul 22, 2025 at 11:16:15AM +0200, Jiri Slaby wrote:
On 22. 07. 25, 10: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 vulnerability by ensuring the descriptor size is greater than or equal to 608 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 [ 13.673297] Read of size 1 at addr ffff888103df39df by task kworker/0:1/10 [ 13.673297] [ 13.673297] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.15.0-00005-gec5d573d83f4-dirty #3 [ 13.673297] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/04 [ 13.673297] Call Trace: [ 13.673297] <TASK> [ 13.673297] dump_stack_lvl+0x5f/0x80 [ 13.673297] print_report+0xd1/0x660 [ 13.673297] kasan_report+0xe5/0x120 [ 13.673297] __asan_report_load1_noabort+0x18/0x20 [ 13.673297] mt_report_fixup+0x103/0x110 [ 13.673297] hid_open_report+0x1ef/0x810 [ 13.673297] mt_probe+0x422/0x960 [ 13.673297] hid_device_probe+0x2e2/0x6f0 [ 13.673297] really_probe+0x1c6/0x6b0 [ 13.673297] __driver_probe_device+0x24f/0x310 [ 13.673297] driver_probe_device+0x4e/0x220 [ 13.673297] __device_attach_driver+0x169/0x320 [ 13.673297] bus_for_each_drv+0x11d/0x1b0 [ 13.673297] __device_attach+0x1b8/0x3e0 [ 13.673297] device_initial_probe+0x12/0x20 [ 13.673297] bus_probe_device+0x13d/0x180 [ 13.673297] device_add+0xe3a/0x1670 [ 13.673297] hid_add_device+0x31d/0xa40 [...]
Fixes: c8000deb6836 ("HID: multitouch: Add support for GT7868Q") Cc: stable@vger.kernel.org Signed-off-by: Qasim Ijaz qasdev00@gmail.com Reviewed-by: Dmitry Savin envelsavinds@gmail.com
drivers/hid/hid-multitouch.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 7ac8e16e6158..af4abe3ba410 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -1461,18 +1461,25 @@ 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 (rdesc[607] == 0x15) {rdesc[607] = 0x25;dev_info(&hdev->dev,"GT7868Q report descriptor fixup is applied.\n");
if (*size >= 608) {if (rdesc[607] == 0x15) {rdesc[607] = 0x25;dev_info(&hdev->dev,"GT7868Q report descriptor fixup is applied.\n");} else {dev_info(&hdev->dev,"The byte is not expected for fixing the report descriptor. \It's possible that the touchpad firmware is not suitable for applying the fix. \got: %x\n",This is wrong. You have all the spaces/tabs in the string now. Drop all the backslashes, and open and close the string on every line.
rdesc[607]);}As this is superlong and superindented, perhaps introduce a new function for these devices?
} else { dev_info( &hdev->dev,
"The byte is not expected for fixing the report descriptor. \-It's possible that the touchpad firmware is not suitable for applying the fix. \ -got: %x\n",
This was horrid too, yeah.
rdesc[607]);
"GT7868Q fixup: report descriptor only %u bytes, skipping\n",A predicate missing. Eg. "has only", or "is only".
Thanks for the feedback Jiri, I took the advice on board, is something like this better?
Definitely.
static const __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *size) { 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( &hdev->dev, "GT7868Q fixup: report descriptor is only %u bytes, skipping\n", *size); return rdesc; } if (rdesc[607] == 0x15) { rdesc[607] = 0x25; dev_info( &hdev->dev, "GT7868Q fixup: report descriptor fixup is applied.\n"); } else { dev_info(&hdev->dev, "GT7868Q fixup: offset 607 is %x (expected 0x15), " "descriptor may be malformed, skipping\n", rdesc[607]); } } return rdesc; }
the key changes I made are:
- Move size check to the top, this way the indentation level is decent
- get rid of message backslashes
- shorten the fixup failure message when rdesc[607] is not 0x15 and make it a bit clearer since this message was the longest - just a minor cleanup
- added "is only %u bytes" as you suggested
if this is all good I can send v2.
I would invert the conditions. So the code would look like:
static bool goodix_needs_fixup(hdev) { if (hdev->vendor != I2C_VENDOR_ID_GOODIX) return false;
return hdev->product == I2C_DEVICE_ID_GOODIX_01E8 || hdev->product == I2C_DEVICE_ID_GOODIX_01E9; }
static const __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *size) { if (!goodix_needs_fixup(hdev)) return rdesc;
if (*size < 608) { dev_info(&hdev->dev, "GT7868Q fixup: report descriptor is only %u bytes, skipping\n", *size); return rdesc; }
if (rdesc[607] != 0x15) { dev_info(&hdev->dev, "GT7868Q fixup: offset 607 is %x (expected 0x15), descriptor may be malformed, skipping\n", rdesc[607]); return rdesc; }
rdesc[607] = 0x25; dev_info(&hdev->dev, "GT7868Q fixup: report descriptor fixup is applied.\n");
return rdesc; }
thanks,
On Wed, Jul 23, 2025 at 06:40:52AM +0200, Jiri Slaby wrote:
On 22. 07. 25, 13:19, Qasim Ijaz wrote:
On Tue, Jul 22, 2025 at 11:16:15AM +0200, Jiri Slaby wrote:
On 22. 07. 25, 10: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 vulnerability by ensuring the descriptor size is greater than or equal to 608 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 [ 13.673297] Read of size 1 at addr ffff888103df39df by task kworker/0:1/10 [ 13.673297] [ 13.673297] CPU: 0 UID: 0 PID: 10 Comm: kworker/0:1 Not tainted 6.15.0-00005-gec5d573d83f4-dirty #3 [ 13.673297] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/04 [ 13.673297] Call Trace: [ 13.673297] <TASK> [ 13.673297] dump_stack_lvl+0x5f/0x80 [ 13.673297] print_report+0xd1/0x660 [ 13.673297] kasan_report+0xe5/0x120 [ 13.673297] __asan_report_load1_noabort+0x18/0x20 [ 13.673297] mt_report_fixup+0x103/0x110 [ 13.673297] hid_open_report+0x1ef/0x810 [ 13.673297] mt_probe+0x422/0x960 [ 13.673297] hid_device_probe+0x2e2/0x6f0 [ 13.673297] really_probe+0x1c6/0x6b0 [ 13.673297] __driver_probe_device+0x24f/0x310 [ 13.673297] driver_probe_device+0x4e/0x220 [ 13.673297] __device_attach_driver+0x169/0x320 [ 13.673297] bus_for_each_drv+0x11d/0x1b0 [ 13.673297] __device_attach+0x1b8/0x3e0 [ 13.673297] device_initial_probe+0x12/0x20 [ 13.673297] bus_probe_device+0x13d/0x180 [ 13.673297] device_add+0xe3a/0x1670 [ 13.673297] hid_add_device+0x31d/0xa40 [...]
Fixes: c8000deb6836 ("HID: multitouch: Add support for GT7868Q") Cc: stable@vger.kernel.org Signed-off-by: Qasim Ijaz qasdev00@gmail.com Reviewed-by: Dmitry Savin envelsavinds@gmail.com
drivers/hid/hid-multitouch.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index 7ac8e16e6158..af4abe3ba410 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -1461,18 +1461,25 @@ 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 (rdesc[607] == 0x15) {rdesc[607] = 0x25;dev_info(&hdev->dev,"GT7868Q report descriptor fixup is applied.\n");
if (*size >= 608) {if (rdesc[607] == 0x15) {rdesc[607] = 0x25;dev_info(&hdev->dev,"GT7868Q report descriptor fixup is applied.\n");} else {dev_info(&hdev->dev,"The byte is not expected for fixing the report descriptor. \It's possible that the touchpad firmware is not suitable for applying the fix. \got: %x\n",This is wrong. You have all the spaces/tabs in the string now. Drop all the backslashes, and open and close the string on every line.
rdesc[607]);}As this is superlong and superindented, perhaps introduce a new function for these devices?
} else { dev_info( &hdev->dev,
"The byte is not expected for fixing the report descriptor. \-It's possible that the touchpad firmware is not suitable for applying the fix. \ -got: %x\n",
This was horrid too, yeah.
rdesc[607]);
"GT7868Q fixup: report descriptor only %u bytes, skipping\n",A predicate missing. Eg. "has only", or "is only".
Thanks for the feedback Jiri, I took the advice on board, is something like this better?
Definitely.
static const __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *size) { 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( &hdev->dev, "GT7868Q fixup: report descriptor is only %u bytes, skipping\n", *size); return rdesc; } if (rdesc[607] == 0x15) { rdesc[607] = 0x25; dev_info( &hdev->dev, "GT7868Q fixup: report descriptor fixup is applied.\n"); } else { dev_info(&hdev->dev, "GT7868Q fixup: offset 607 is %x (expected 0x15), " "descriptor may be malformed, skipping\n", rdesc[607]); } } return rdesc; }
the key changes I made are:
- Move size check to the top, this way the indentation level is decent
- get rid of message backslashes
- shorten the fixup failure message when rdesc[607] is not 0x15 and make it a bit clearer since this message was the longest - just a minor cleanup
- added "is only %u bytes" as you suggested
if this is all good I can send v2.
I would invert the conditions. So the code would look like:
static bool goodix_needs_fixup(hdev) { if (hdev->vendor != I2C_VENDOR_ID_GOODIX) return false;
return hdev->product == I2C_DEVICE_ID_GOODIX_01E8 || hdev->product == I2C_DEVICE_ID_GOODIX_01E9; }
static const __u8 *mt_report_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *size) { if (!goodix_needs_fixup(hdev)) return rdesc;
if (*size < 608) { dev_info(&hdev->dev, "GT7868Q fixup: report descriptor is only %u bytes, skipping\n", *size); return rdesc; }
if (rdesc[607] != 0x15) { dev_info(&hdev->dev, "GT7868Q fixup: offset 607 is %x (expected 0x15), descriptor may be malformed, skipping\n", rdesc[607]); return rdesc; }
rdesc[607] = 0x25; dev_info(&hdev->dev, "GT7868Q fixup: report descriptor fixup is applied.\n");
return rdesc; }
Thanks Jiri, this looks good. I think Ill split this into 2 patches one for fixing the OOB with a size check and the second for a general function cleanup. Hope that sounds good.
Thanks qasim
thanks,
js suse labs
linux-stable-mirror@lists.linaro.org
-
Dmitry Savin -
Jiri Slaby -
Qasim Ijaz