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

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

[Linkai Gong]

The 16-bit PIP length is used as the i2c_master_recv() size
into a stack struct of 10 contacts. The 127-byte check runs
after that read. Also clamp the 5-bit contact count.

Fixes: 6972a859601a ("Input: cyapa - add gen5 trackpad device basic functions support")
Signed-off-by: Linkai Gong <gonglinkai@kylinos.cn>
---
 drivers/input/mouse/cyapa_gen5.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/input/mouse/cyapa_gen5.c b/drivers/input/mouse/cyapa_gen5.c
index 59f6e97d5482..0c8bc37ab3fd 100644
--- a/drivers/input/mouse/cyapa_gen5.c
+++ b/drivers/input/mouse/cyapa_gen5.c
@@ -2750,12 +2750,14 @@ static void cyapa_pip_report_touches(struct cyapa *cyapa,
 	struct input_dev *input = cyapa->input;
 	unsigned int touch_num;
 	int i;
 
 	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]);
 
 	input_mt_sync_frame(input);
@@ -2781,13 +2783,14 @@ int cyapa_pip_irq_handler(struct cyapa *cyapa)
 		dev_err(dev, "failed to read length bytes, (%d)\n", ret);
 		return -EINVAL;
 	}
 
 	report_len = get_unaligned_le16(
 			&report_data.report_head[PIP_RESP_LENGTH_OFFSET]);
-	if (report_len < PIP_RESP_LENGTH_SIZE) {
+	if (report_len < PIP_RESP_LENGTH_SIZE ||
+	    report_len > sizeof(report_data)) {
 		/* Invalid length or internal reset happened. */
 		dev_err(dev, "invalid report_len=%d. bytes: %02x %02x\n",
 			report_len, report_data.report_head[0],
 			report_data.report_head[1]);
 		return -EINVAL;
 	}
-- 
2.25.1

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

[sashiko-bot]

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