Re: [PATCH] Input: cyapa - validate unsupported PIP response length

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Stack buffer overflow in `cyapa_pip_irq_handler` when reading touch reports.
- [High] Stack out-of-bounds read in `cyapa_gen5_read_panel_scan_raw_data` during raw data parsing.
- [High] Stack out-of-bounds read and kernel infinite loop in `cyapa_gen5_read_idac_data`.
--

commit 577a902f2ff46d9f3cceff655a89f68889610a56
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

Input: cyapa - validate unsupported PIP response length

cyapa_sort_tsg_pip_app_resp_data() accepts PIP application
responses after checking PIP_MIN_APP_RESP_LENGTH bytes.  That covers
the application command byte at offset 4, but the unsupported-command
path also reads the status byte at PIP_RESP_STATUS_OFFSET.

A short response can advertise PIP_UNSUPPORTED_CMD_RESP_LENGTH in the
response header while the supplied buffer still contains only the generic
minimum response.  Validate the unsupported response length before reading
the status byte.

> diff --git a/drivers/input/mouse/cyapa_gen5.c b/drivers/input/mouse/cyapa_gen5.c
> index 59f6e97d54827..733da6d324331 100644
> --- a/drivers/input/mouse/cyapa_gen5.c
> +++ b/drivers/input/mouse/cyapa_gen5.c
> @@ -686,6 +686,7 @@ bool cyapa_sort_tsg_pip_app_resp_data(struct cyapa *cyapa,
>  		u8 *data, int len)
>  {
>  	struct cyapa_pip_cmd_states *pip = &cyapa->cmd_states.pip;
> +	u8 cmd_code;
>  	int resp_len;
>  
>  	if (!data || len < PIP_MIN_APP_RESP_LENGTH)

[ ... ]

>  		/* Read mutual global idac or local mutual/self PWC data. */
>  		offset += read_len;
>  		for (i = 10; i < (read_len + GEN5_RESP_DATA_STRUCTURE_OFFSET);
>  				i += *data_size) {
>  			value = cyapa_parse_structure_data(resp_data[9],
>  					&resp_data[i], *data_size);

[Severity: High]
This is a pre-existing issue, but does this loop in
cyapa_gen5_read_idac_data() risk an infinite loop or out-of-bounds read?

If a malicious I2C peripheral sets the data size (derived from resp_data[9])
to 0, wouldn't the loop fail to increment i and hang the CPU?

Additionally, doesn't blindly trusting read_len (derived from resp_data[7:8])
for the loop bound risk reading past the bounds of the 256-byte resp_data
stack array?

[ ... ]

> 		read_elements = get_unaligned_le16(&resp_data[7]);
> 		if (read_elements == 0)
> 			break;
> 
> 		data_size = (resp_data[9] & GEN5_PWC_DATA_ELEMENT_SIZE_MASK);
> 		offset += read_elements;
> 		if (read_elements) {
> 			for (i = GEN5_RESP_DATA_STRUCTURE_OFFSET;
> 			     i < (read_elements * data_size +
> 					GEN5_RESP_DATA_STRUCTURE_OFFSET);
> 			     i += data_size) {
> 				value = cyapa_parse_structure_data(resp_data[9],
> 						&resp_data[i], data_size);

[Severity: High]
This isn't a bug introduced by this patch, but does this risk an out-of-bounds
read on the kernel stack in cyapa_gen5_read_panel_scan_raw_data()?

If the device returns a large read_elements value, the loop condition appears
to allow cyapa_parse_structure_data() to read far beyond the bounds of the
256-byte resp_data stack array.

[ ... ]

> int cyapa_pip_irq_handler(struct cyapa *cyapa)
> {
> 	struct device *dev = &cyapa->client->dev;
> 	struct cyapa_pip_report_data report_data;
> 	unsigned int report_len;
> 	int ret;
> 
> 	if (!cyapa_is_pip_app_mode(cyapa)) {
> 		dev_err(dev, "invalid device state, gen=%d, state=0x%02x\n",
> 			cyapa->gen, cyapa->state);
> 		return -EINVAL;
> 	}
> 
> 	ret = cyapa_i2c_pip_read(cyapa, (u8 *)&report_data,
> 			PIP_RESP_LENGTH_SIZE);

[ ... ]

> 	/* Idle, no data for report. */
> 	if (report_len == PIP_RESP_LENGTH_SIZE)
> 		return 0;
> 
> 	ret = cyapa_i2c_pip_read(cyapa, (u8 *)&report_data, report_len);

[Severity: Critical]
This is a pre-existing issue, but could this lead to a stack buffer overflow
in cyapa_pip_irq_handler()?

The report_data structure is 127 bytes on the stack, but an untrusted
report_len from the device is used in cyapa_i2c_pip_read(). Since the read
wrapper only caps the length to 256 bytes (CYAPA_REG_MAP_SIZE), can't a
compromised peripheral supply a length that overflows the buffer and corrupts
the stack?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/2026063008017969.1-ccfa108-0008-Input-cyapa---validate-unsu-pengpeng@iscas.ac.cn?part=1