Re: [PATCH] Input: cyapa - cap the PIP report length before the I2C read

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the driver processes up to `touch_num` touch records from `report_data.touch_records` without verifying that the `report_len` read from the I2C device is actually large enough to contain them. If `report_len` is smaller than required but `touch_num` is non-zero, it reads uninitialized kernel stack memory and reports it to userspace as input events.
--

commit a3170cabcda3be9e64d6692bbb6c52bf82b02d07
Author: Linkai Gong <gonglinkai@kylinos.cn>

Input: cyapa - cap the PIP report length before the I2C read

This commit fixes an issue where the PIP report length could exceed the
size of the receiving buffer. It clamps the report length to the size of the
report_data structure and bounds the number of touch records.

> diff --git a/drivers/input/mouse/cyapa_gen5.c b/drivers/input/mouse/cyapa_gen5.c
> index 59f6e97d54827..0c8bc37ab3fd4 100644
> --- a/drivers/input/mouse/cyapa_gen5.c
> +++ b/drivers/input/mouse/cyapa_gen5.c
> @@ -2753,6 +2753,8 @@ static void cyapa_pip_report_touches(struct cyapa *cyapa,
>  
>  	touch_num = report_data->report_head[PIP_NUMBER_OF_TOUCH_OFFSET] &
>  			PIP_NUMBER_OF_TOUCH_MASK;
> +	if (touch_num > ARRAY_SIZE(report_data->touch_records))
> +		touch_num = ARRAY_SIZE(report_data->touch_records);
>  
>  	for (i = 0; i < touch_num; i++)
>  		cyapa_pip_report_slot_data(cyapa,
>  			&report_data->touch_records[i]);

[Severity: High]
This isn't a bug introduced by this patch, but does this loop potentially
process uninitialized stack memory?

If a malfunctioning or malicious I2C device provides a small report_len
(e.g., 7 bytes) but a non-zero touch_num in the header, the loop iterates
up to touch_num times.

Because cyapa_pip_irq_handler() allocates an uninitialized stack buffer
for report_data and only reads report_len bytes into it:

int cyapa_pip_irq_handler(struct cyapa *cyapa)
{
	struct cyapa_pip_report_data report_data;
	[ ... ]
	ret = cyapa_i2c_pip_read(cyapa, (u8 *)&report_data, report_len);
}

This leaves the remainder of the touch_records array uninitialized.

Processing these records here could leak uninitialized kernel stack memory
and cause erratic phantom input events to be reported to userspace.

Should there be an additional check to verify that report_len is actually
large enough to contain touch_num records?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903075026.1069672-1-gonglinkai@kylinos.cn?part=1