* [PATCH] Input: hynitron_cstxxx: reject out-of-range finger IDs
@ 2026-08-04 2:03 Jianing Li
2026-08-04 2:17 ` sashiko-bot
2026-08-04 3:13 ` [PATCH v2] Input: hynitron_cstxxx: validate touch count and " Jianing Li
0 siblings, 2 replies; 5+ messages in thread
From: Jianing Li @ 2026-08-04 2:03 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, stable
The driver allocates max_touch_num input slots, which are indexed from
zero through max_touch_num - 1. The current check allows a finger ID
equal to max_touch_num to reach input_mt_report_slot_state(), resulting
in an out-of-bounds slot access.
Reject IDs at the upper bound before using them as slot indexes.
The V821 Avaota F1 board configures this driver with one touch slot, so
finger ID 1 is already invalid on that device.
Fixes: 66603243f528 ("Input: add driver for Hynitron cstxxx touchscreens")
Cc: stable@vger.kernel.org
Signed-off-by: Jianing Li <m13940358460@163.com>
---
drivers/input/touchscreen/hynitron_cstxxx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/hynitron_cstxxx.c b/drivers/input/touchscreen/hynitron_cstxxx.c
index f6139b1a8681..05e877b0d877 100644
--- a/drivers/input/touchscreen/hynitron_cstxxx.c
+++ b/drivers/input/touchscreen/hynitron_cstxxx.c
@@ -333,7 +333,7 @@ static void cst3xx_touch_report(struct i2c_client *client)
sw = (buf[idx] & 0x0f) >> 1;
finger_id = (buf[idx] >> 4) & 0x0f;
/* Sanity check we don't have more fingers than we expect */
- if (ts_data->chip->max_touch_num < finger_id) {
+ if (finger_id >= ts_data->chip->max_touch_num) {
dev_err(&client->dev, "cst3xx touch read failure\n");
break;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] Input: hynitron_cstxxx: reject out-of-range finger IDs
2026-08-04 2:03 [PATCH] Input: hynitron_cstxxx: reject out-of-range finger IDs Jianing Li
@ 2026-08-04 2:17 ` sashiko-bot
2026-08-04 3:13 ` [PATCH v2] Input: hynitron_cstxxx: validate touch count and " Jianing Li
1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-08-04 2:17 UTC (permalink / raw)
To: Jianing Li; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing bounds check on `touch_cnt` leads to out-of-bounds stack memory reads.
--
--- Patch [1]: [PATCH] Input: hynitron_cstxxx: reject out-of-range finger IDs ---
commit 91e0c941ddcc6d0f4ff8d05bd12420502e2416a4
Author: Jianing Li <m13940358460@163.com>
Input: hynitron_cstxxx: reject out-of-range finger IDs
The driver allocates max_touch_num input slots, which are indexed from
zero through max_touch_num - 1. This commit fixes an out-of-bounds access
by rejecting finger IDs that are greater than or equal to max_touch_num
before using them as slot indexes.
> diff --git a/drivers/input/touchscreen/hynitron_cstxxx.c b/drivers/input/touchscreen/hynitron_cstxxx.c
> index f6139b1a86811..05e877b0d8776 100644
> --- a/drivers/input/touchscreen/hynitron_cstxxx.c
> +++ b/drivers/input/touchscreen/hynitron_cstxxx.c
> @@ -334,7 +334,7 @@ static void cst3xx_touch_report(struct i2c_client *client)
> finger_id = (buf[idx] >> 4) & 0x0f;
>
> /* Sanity check we don't have more fingers than we expect */
> - if (ts_data->chip->max_touch_num < finger_id) {
> + if (finger_id >= ts_data->chip->max_touch_num) {
[Severity: High]
This is a pre-existing issue, but while we are checking for out-of-range
values in cst3xx_touch_report(), is there a missing bounds check on
touch_cnt earlier in this function?
The touch_cnt is extracted from the 28-byte fixed-size buf array:
touch_cnt = buf[5] & CST3XX_TOUCH_COUNT_MASK;
If the device returns a touch_cnt greater than 5, wouldn't the end_byte
calculation result in an out-of-bounds stack memory read when checking the
last touch slot?
if (touch_cnt > 1) {
end_byte = touch_cnt * 5 + 2;
if (buf[end_byte] != CST3XX_TOUCH_DATA_CHK_VAL) {
For example, a touch_cnt of 6 or more would evaluate to an end_byte of 32
or greater, reading past the end of the 28-byte buf array. Similarly, the
subsequent coordinate extraction loop would also sequentially read past the
end of buf.
Can this out-of-bounds read occur if the device firmware malfunctions or
returns an unexpectedly large touch count?
> dev_err(&client->dev, "cst3xx touch read failure\n");
> break;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260804020314.2082-1-m13940358460@163.com?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2] Input: hynitron_cstxxx: validate touch count and finger IDs
2026-08-04 2:03 [PATCH] Input: hynitron_cstxxx: reject out-of-range finger IDs Jianing Li
2026-08-04 2:17 ` sashiko-bot
@ 2026-08-04 3:13 ` Jianing Li
2026-08-04 3:21 ` sashiko-bot
2026-08-04 4:51 ` Dmitry Torokhov
1 sibling, 2 replies; 5+ messages in thread
From: Jianing Li @ 2026-08-04 3:13 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, stable
The driver allocates max_touch_num input slots, which are indexed from
zero through max_touch_num - 1. The current check allows a finger ID
equal to max_touch_num to reach input_mt_report_slot_state(), resulting
in an out-of-bounds slot access.
The touch count is read from the controller's report and is used to
index the fixed-size report buffer without first checking its range.
Reject counts larger than the supported number of touch slots before
checking the trailing byte or parsing touch data.
Reject IDs at the upper bound before using them as slot indexes.
The V821 Avaota F1 board configures the vendor driver with one touch
slot, so finger ID 1 is already invalid on that device.
Fixes: 66603243f528 ("Input: add driver for Hynitron cstxxx touchscreens")
Cc: stable@vger.kernel.org
Signed-off-by: Jianing Li <m13940358460@163.com>
---
drivers/input/touchscreen/hynitron_cstxxx.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/hynitron_cstxxx.c b/drivers/input/touchscreen/hynitron_cstxxx.c
index f6139b1a8681..05e877b0d877 100644
--- a/drivers/input/touchscreen/hynitron_cstxxx.c
+++ b/drivers/input/touchscreen/hynitron_cstxxx.c
@@ -293,6 +293,11 @@ static void cst3xx_touch_report(struct i2c_client *client)
touch_cnt = buf[5] & CST3XX_TOUCH_COUNT_MASK;
+ if (touch_cnt > ts_data->chip->max_touch_num) {
+ dev_err(&client->dev, "cst3xx touch read failure\n");
+ return;
+ }
+
/*
* Check the check bit of the last touch slot. The check bit is
* always present after touch point 1 for valid data, and then
* appears as the last byte after all other touch data.
*/
@@ -333,7 +338,7 @@ static void cst3xx_touch_report(struct i2c_client *client)
sw = (buf[idx] & 0x0f) >> 1;
finger_id = (buf[idx] >> 4) & 0x0f;
/* Sanity check we don't have more fingers than we expect */
- if (ts_data->chip->max_touch_num < finger_id) {
+ if (finger_id >= ts_data->chip->max_touch_num) {
dev_err(&client->dev, "cst3xx touch read failure\n");
break;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] Input: hynitron_cstxxx: validate touch count and finger IDs
2026-08-04 3:13 ` [PATCH v2] Input: hynitron_cstxxx: validate touch count and " Jianing Li
@ 2026-08-04 3:21 ` sashiko-bot
2026-08-04 4:51 ` Dmitry Torokhov
1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-08-04 3:21 UTC (permalink / raw)
To: Jianing Li; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver passes stack-allocated buffers directly to I2C transfer functions, which violates DMA mapping rules and can cause kernel panics or data corruption.
--
--- Patch [1]: [PATCH v2] Input: hynitron_cstxxx: validate touch count and finger IDs ---
commit cf5d1ec7af0f8cb9ee23a1f75f59f7e64afb8fe7
Author: Jianing Li <m13940358460@163.com>
Input: hynitron_cstxxx: validate touch count and finger IDs
This commit adds bounds checking for the touch count and finger ID read
from the controller. It ensures that neither exceeds max_touch_num,
preventing out-of-bounds slot accesses during input reporting.
> diff --git a/drivers/input/touchscreen/hynitron_cstxxx.c b/drivers/input/touchscreen/hynitron_cstxxx.c
> index f6139b1a86811..5f2737fe99026 100644
> --- a/drivers/input/touchscreen/hynitron_cstxxx.c
> +++ b/drivers/input/touchscreen/hynitron_cstxxx.c
> @@ -312,6 +312,11 @@ static void cst3xx_touch_report(struct i2c_client *client)
> return;
>
> touch_cnt = buf[5] & CST3XX_TOUCH_COUNT_MASK;
[Severity: High]
This is a pre-existing issue, but I noticed that buf is a stack-allocated
array in cst3xx_touch_report() that is passed to i2c_transfer() via
cst3xx_i2c_read_register() earlier in the function.
If the I2C host controller maps the buffers for DMA, passing a stack-allocated
buffer can lead to memory corruption, kernel panics, or DMA API debug warnings
because stack memory is not DMA-safe (especially with virtual kernel stacks
enabled).
Looking at the rest of drivers/input/touchscreen/hynitron_cstxxx.c, similar
stack allocations occur in other functions:
cst3xx_i2c_read_register() allocates __le16 buf
cst3xx_firmware_info() allocates unsigned char buf[4]
cst3xx_bootloader_enter() allocates unsigned char buf[3]
cst3xx_finish_touch_read() allocates unsigned char buf[3]
Should these buffers be heap-allocated or moved to a DMA-safe allocation
within the device structure to prevent DMA corruption?
> + if (touch_cnt > ts_data->chip->max_touch_num) {
> + dev_err(&client->dev, "cst3xx touch read failure\n");
> + return;
> + }
> +
> /*
> * Check the check bit of the last touch slot. The check bit is
> * always present after touch point 1 for valid data, and then
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260804031339.2379-1-m13940358460@163.com?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] Input: hynitron_cstxxx: validate touch count and finger IDs
2026-08-04 3:13 ` [PATCH v2] Input: hynitron_cstxxx: validate touch count and " Jianing Li
2026-08-04 3:21 ` sashiko-bot
@ 2026-08-04 4:51 ` Dmitry Torokhov
1 sibling, 0 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2026-08-04 4:51 UTC (permalink / raw)
To: Jianing Li; +Cc: linux-input, stable
Hi Jianing,
On Tue, Aug 04, 2026 at 11:13:39AM +0800, Jianing Li wrote:
> The driver allocates max_touch_num input slots, which are indexed from
> zero through max_touch_num - 1. The current check allows a finger ID
> equal to max_touch_num to reach input_mt_report_slot_state(), resulting
> in an out-of-bounds slot access.
This is not quite correct. There is no OOB access because input core
validates reported slot ids and wil refuse switching to a slot that is
outside of limits.
Still, the driver should not be reporting such packets.
>
> The touch count is read from the controller's report and is used to
> index the fixed-size report buffer without first checking its range.
> Reject counts larger than the supported number of touch slots before
> checking the trailing byte or parsing touch data.
>
> Reject IDs at the upper bound before using them as slot indexes.
>
> The V821 Avaota F1 board configures the vendor driver with one touch
> slot, so finger ID 1 is already invalid on that device.
>
> Fixes: 66603243f528 ("Input: add driver for Hynitron cstxxx touchscreens")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jianing Li <m13940358460@163.com>
> ---
> drivers/input/touchscreen/hynitron_cstxxx.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/hynitron_cstxxx.c b/drivers/input/touchscreen/hynitron_cstxxx.c
> index f6139b1a8681..05e877b0d877 100644
> --- a/drivers/input/touchscreen/hynitron_cstxxx.c
> +++ b/drivers/input/touchscreen/hynitron_cstxxx.c
> @@ -293,6 +293,11 @@ static void cst3xx_touch_report(struct i2c_client *client)
> touch_cnt = buf[5] & CST3XX_TOUCH_COUNT_MASK;
> + if (touch_cnt > ts_data->chip->max_touch_num) {
> + dev_err(&client->dev, "cst3xx touch read failure\n");
We need to have better error message.
> + return;
> + }
> +
> /*
> * Check the check bit of the last touch slot. The check bit is
> * always present after touch point 1 for valid data, and then
> * appears as the last byte after all other touch data.
> */
> @@ -333,7 +338,7 @@ static void cst3xx_touch_report(struct i2c_client *client)
> sw = (buf[idx] & 0x0f) >> 1;
> finger_id = (buf[idx] >> 4) & 0x0f;
>
> /* Sanity check we don't have more fingers than we expect */
> - if (ts_data->chip->max_touch_num < finger_id) {
> + if (finger_id >= ts_data->chip->max_touch_num) {
> dev_err(&client->dev, "cst3xx touch read failure\n");
Same here.
> break;
Not your change but break here results in finalizing incomplete packet.
It is better to abort (return) and wait for proper packet without
invalid fingers.
> }
I made adjustments and applied, thank you.
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-04 4:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-04 2:03 [PATCH] Input: hynitron_cstxxx: reject out-of-range finger IDs Jianing Li
2026-08-04 2:17 ` sashiko-bot
2026-08-04 3:13 ` [PATCH v2] Input: hynitron_cstxxx: validate touch count and " Jianing Li
2026-08-04 3:21 ` sashiko-bot
2026-08-04 4:51 ` Dmitry Torokhov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox