Re: [PATCH v2] Input: hynitron_cstxxx: validate touch count and finger IDs

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: [email protected]
> Signed-off-by: Jianing Li <[email protected]>
> ---
>  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