Re: [PATCH 2/8] HID: alps: validate report size in raw_event callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] HID: alps: off-by-two report size check in u1_raw_event()
--

commit 5284d6e143901f92388237eea63f898c2ce83a7f
Author: Tristan Madani <tristan@talencesecurity.com>

HID: alps: validate report size in raw_event callbacks

This commit adds report buffer size validation to t4_raw_event() and
u1_raw_event() before accessing report fields.

> diff --git a/drivers/hid/hid-alps.c b/drivers/hid/hid-alps.c
> index 0556cb5645ebb..dee7d44d6f912 100644
> --- a/drivers/hid/hid-alps.c
> +++ b/drivers/hid/hid-alps.c
[ ... ]
> @@ -370,6 +378,8 @@ static int u1_raw_event(struct alps_dev *hdata, u8 *data, int size)
>  		break;
>  	case U1_ABSOLUTE_REPORT_ID:
>  	case U1_ABSOLUTE_REPORT_ID_SECD:
> +		if (size < hdata->max_fingers * 5 + 5)
> +			return 0;
>  		for (i = 0; i < hdata->max_fingers; i++) {
>  			u8 *contact = &data[i * 5];

[Severity: Medium]
Does this check in u1_raw_event() reject valid 28-byte 5-finger touchpad
reports when receiving U1_ABSOLUTE_REPORT_ID or U1_ABSOLUTE_REPORT_ID_SECD?

Inside the loop, contact is set to &data[i * 5] and the code reads:

	for (i = 0; i < hdata->max_fingers; i++) {
		u8 *contact = &data[i * 5];

		x = get_unaligned_le16(contact + 3);
		y = get_unaligned_le16(contact + 5);
		z = contact[7] & 0x7F;

The report has a 3-byte header (data[0] report ID, data[1] buttons, data[2]
finger count) followed by hdata->max_fingers 5-byte contact entries starting
at data[3]. On the final iteration (i = hdata->max_fingers - 1), the last
byte accessed is contact[7], which is at index:

	(hdata->max_fingers - 1) * 5 + 7 = hdata->max_fingers * 5 + 2

For a 5-finger touchpad (hdata->max_fingers == 5), the last byte is data[27]
(matching the Packet Format table in Documentation/hid/hid-alps.rst), so the
report is 28 bytes long. Requiring hdata->max_fingers * 5 + 5 demands 30
bytes. Should the bound be hdata->max_fingers * 5 + 3 instead?

[ ... ]

>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018213754.1951187-1-tristmd@gmail.com?part=2