Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] Input: cyapa - validate unsupported PIP response length
@ 2026-06-30  7:19 Pengpeng Hou
  2026-06-30  7:38 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Pengpeng Hou @ 2026-06-30  7:19 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: open list:INPUT KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN...,
	open list, Pengpeng Hou

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.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/input/mouse/cyapa_gen5.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/input/mouse/cyapa_gen5.c b/drivers/input/mouse/cyapa_gen5.c
index 59f6e97..733da6d 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)
@@ -694,13 +695,19 @@ bool cyapa_sort_tsg_pip_app_resp_data(struct cyapa *cyapa,
 	if (data[PIP_RESP_REPORT_ID_OFFSET] == PIP_APP_RESP_REPORT_ID &&
 			data[PIP_RESP_RSVD_OFFSET] == PIP_RESP_RSVD_KEY) {
 		resp_len = get_unaligned_le16(&data[PIP_RESP_LENGTH_OFFSET]);
-		if (GET_PIP_CMD_CODE(data[PIP_RESP_APP_CMD_OFFSET]) == 0x00 &&
-			resp_len == PIP_UNSUPPORTED_CMD_RESP_LENGTH &&
-			data[5] == pip->in_progress_cmd) {
-			/* Unsupported command code */
-			return false;
-		} else if (GET_PIP_CMD_CODE(data[PIP_RESP_APP_CMD_OFFSET]) ==
-				pip->in_progress_cmd) {
+		cmd_code = GET_PIP_CMD_CODE(data[PIP_RESP_APP_CMD_OFFSET]);
+		if (cmd_code == 0x00 &&
+		    resp_len == PIP_UNSUPPORTED_CMD_RESP_LENGTH) {
+			if (len < PIP_UNSUPPORTED_CMD_RESP_LENGTH)
+				return false;
+
+			if (data[PIP_RESP_STATUS_OFFSET] == pip->in_progress_cmd) {
+				/* Unsupported command code */
+				return false;
+			}
+		}
+
+		if (cmd_code == pip->in_progress_cmd) {
 			/* Correct command response received */
 			return true;
 		}


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] Input: cyapa - validate unsupported PIP response length
  2026-06-30  7:19 [PATCH] Input: cyapa - validate unsupported PIP response length Pengpeng Hou
@ 2026-06-30  7:38 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-06-30  7:38 UTC (permalink / raw)
  To: Pengpeng Hou; +Cc: dmitry.torokhov, linux-input

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-30  7:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-30  7:19 [PATCH] Input: cyapa - validate unsupported PIP response length Pengpeng Hou
2026-06-30  7:38 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox