Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH] Input: cyttsp5 - clamp the HID report size before memcpy
@ 2026-09-01 12:26 Linkai Gong
  2026-09-01 12:40 ` sashiko-bot
  2026-09-04 10:31 ` Alistair
  0 siblings, 2 replies; 3+ messages in thread
From: Linkai Gong @ 2026-09-01 12:26 UTC (permalink / raw)
  To: Linus Walleij, Dmitry Torokhov
  Cc: Alistair Francis, Maxime Ripard, Mylène Josserand,
	linux-input, linux-kernel, gonglinkai

The size field comes from the device and is used as the memcpy()
length into response_buf, which is CY_MAX_INPUT bytes.

Fixes: 5b0c03e24a06 ("Input: Add driver for Cypress Generation 5 touchscreen")
Signed-off-by: Linkai Gong <gonglinkai@kylinos.cn>
---
 drivers/input/touchscreen/cyttsp5.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
index 9266c07314be..e878a02dc9b7 100644
--- a/drivers/input/touchscreen/cyttsp5.c
+++ b/drivers/input/touchscreen/cyttsp5.c
@@ -710,6 +710,7 @@ static irqreturn_t cyttsp5_handle_irq(int irq, void *handle)
 		size = 2;
 	} else {
 		report_id = ts->input_buf[2];
+		size = min(size, CY_MAX_INPUT);
 	}
 
 	switch (report_id) {
-- 
2.25.1


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

* Re: [PATCH] Input: cyttsp5 - clamp the HID report size before memcpy
  2026-09-01 12:26 [PATCH] Input: cyttsp5 - clamp the HID report size before memcpy Linkai Gong
@ 2026-09-01 12:40 ` sashiko-bot
  2026-09-04 10:31 ` Alistair
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-01 12:40 UTC (permalink / raw)
  To: Linkai Gong; +Cc: linux-input, dmitry.torokhov

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

Pre-existing issues:
- [High] Incomplete Buffer Overflow Fix (TOCTOU) / Out-of-bounds Read / Integer Underflow
- [High] Out-of-Bounds Read / Missing Length Validation for Touch Data
--

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

Input: cyttsp5 - clamp the HID report size before memcpy

This commit attempts to fix a buffer overflow by clamping the size of the
HID report payload before copying it into the response buffer.

> diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
> index 9266c07314bee..e878a02dc9b75 100644
> --- a/drivers/input/touchscreen/cyttsp5.c
> +++ b/drivers/input/touchscreen/cyttsp5.c
> @@ -710,6 +710,7 @@ static irqreturn_t cyttsp5_handle_irq(int irq, void *handle)
>  		size = 2;
>  	} else {
>  		report_id = ts->input_buf[2];
> +		size = min(size, CY_MAX_INPUT);
>  	}
>  
>  	switch (report_id) {

[Severity: High]
This isn't a bug introduced by this patch, but the patch provides an
incomplete fix. Although the local size variable is clamped here to protect
the memcpy, the unclamped size remains embedded within the copied payload in
ts->input_buf, which is then copied to ts->response_buf.

Later, cyttsp5_validate_cmd_response() reads the original unclamped size
directly from the buffer:

cyttsp5_validate_cmd_response() {
    ...
    size = get_unaligned_le16(&ts->response_buf[0]);
    ...
    if (ts->response_buf[size - 1] != HID_OUTPUT_BL_EOP) {
        ...

If the device provided a size greater than 512, does this result in an
out-of-bounds read?

Additionally, if the size is very small (e.g., 5), the expression size - 7
mathematically underflows when passed to crc_itu_t():

crc = crc_itu_t(0xFFFF, &ts->response_buf[4], size - 7);

Could we also validate the size value stored inside the buffer, or ensure
that the extracted size in cyttsp5_validate_cmd_response() falls within safe
bounds (>= 7 and <= CY_MAX_INPUT)?


[Severity: High]
This is also a pre-existing issue, but while we are looking at bounds checking
in this driver, there appears to be missing validation for the touch data
sizes as well.

In cyttsp5_get_sysinfo_regs(), scd->max_tch is set directly from the device
payload without capping it to fit within the static CY_MAX_INPUT buffer
bounds.

When an interrupt occurs, cyttsp5_mt_attention() extracts num_cur_tch from
the payload. There is no check to guarantee that num_cur_tch multiplied by
TOUCH_REPORT_SIZE fits within the statically allocated 512-byte buffer.

For large values of num_cur_tch, the loop in cyttsp5_get_mt_touches()
calculates pointers that exceed the 512-byte bound:

cyttsp5_get_mt_touches() {
    ...
    for (i = 0; i < num_cur_tch; i++) {
        tch_addr = ts->input_buf + offset + (i * TOUCH_REPORT_SIZE);
        ...

Could this lead to out-of-bounds reads of the ts->input_buf array,
potentially leaking adjacent kernel heap data into userspace via the input
events?

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

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

* Re: [PATCH] Input: cyttsp5 - clamp the HID report size before memcpy
  2026-09-01 12:26 [PATCH] Input: cyttsp5 - clamp the HID report size before memcpy Linkai Gong
  2026-09-01 12:40 ` sashiko-bot
@ 2026-09-04 10:31 ` Alistair
  1 sibling, 0 replies; 3+ messages in thread
From: Alistair @ 2026-09-04 10:31 UTC (permalink / raw)
  To: Linkai Gong, Linus Walleij, Dmitry Torokhov
  Cc: Maxime Ripard, Mylène Josserand, linux-input, linux-kernel

On Tue, 2026-09-01 at 20:26 +0800, Linkai Gong wrote:
> The size field comes from the device and is used as the memcpy()
> length into response_buf, which is CY_MAX_INPUT bytes.
> 
> Fixes: 5b0c03e24a06 ("Input: Add driver for Cypress Generation 5
> touchscreen")
> Signed-off-by: Linkai Gong <gonglinkai@kylinos.cn>

Reviewed-by: Alistair Francis <alistair@alistair23.me>

Alistair

> ---
>  drivers/input/touchscreen/cyttsp5.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/touchscreen/cyttsp5.c
> b/drivers/input/touchscreen/cyttsp5.c
> index 9266c07314be..e878a02dc9b7 100644
> --- a/drivers/input/touchscreen/cyttsp5.c
> +++ b/drivers/input/touchscreen/cyttsp5.c
> @@ -710,6 +710,7 @@ static irqreturn_t cyttsp5_handle_irq(int irq,
> void *handle)
>  		size = 2;
>  	} else {
>  		report_id = ts->input_buf[2];
> +		size = min(size, CY_MAX_INPUT);
>  	}
>  
>  	switch (report_id) {

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

end of thread, other threads:[~2026-09-04 10:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-01 12:26 [PATCH] Input: cyttsp5 - clamp the HID report size before memcpy Linkai Gong
2026-09-01 12:40 ` sashiko-bot
2026-09-04 10:31 ` Alistair

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