Re: [PATCH] Input: focaltech - use signed coordinates to prevent underflow

[Hans de Goede]

Hi,

On 3-Aug-26 03:22, Dmitry Torokhov wrote:
> focaltech_finger_state stores finger coordinates x and y as unsigned
> int. When processing relative packets, negative deltas can cause
> unsigned integer underflow if the finger moves past the left or bottom
> boundary of the touchpad, wrapping the coordinates to values near
> UINT_MAX.
> 
> When clamping the coordinates in focaltech_report_state(), these
> underflowed values are clamped against priv->x_max / priv->y_max instead
> of 0, causing the cursor to jump erratically to the opposite edge of the
> touchpad.
> 
> Change the coordinate variables and limits to signed int so that
> negative values resulting from relative movements clamp correctly to 0.
> 
> Fixes: 05be1d079ec0 ("Input: psmouse - support for the FocalTech PS/2 protocol extensions")
> Reported-by: sashiko-bot@xxxxxxxxxx
> Assisted-by: Antigravity:gemini-3.6-flash
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@xxxxxxxxxxxxxxxx>

Regards,

Hans


> ---
>  drivers/input/mouse/focaltech.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/mouse/focaltech.c b/drivers/input/mouse/focaltech.c
> index d3ad4af5aa09..c6f6540e3e29 100644
> --- a/drivers/input/mouse/focaltech.c
> +++ b/drivers/input/mouse/focaltech.c
> @@ -78,8 +78,8 @@ struct focaltech_finger_state {
>  	 * Absolute position (from the bottom left corner) of the
>  	 * finger.
>  	 */
> -	unsigned int x;
> -	unsigned int y;
> +	int x;
> +	int y;
>  };
>  
>  /*
> @@ -108,7 +108,7 @@ struct focaltech_hw_state {
>  };
>  
>  struct focaltech_data {
> -	unsigned int x_max, y_max;
> +	int x_max, y_max;
>  	struct focaltech_hw_state state;
>  };
>  
> @@ -126,14 +126,14 @@ static void focaltech_report_state(struct psmouse *psmouse)
>  		input_mt_slot(dev, i);
>  		input_mt_report_slot_state(dev, MT_TOOL_FINGER, active);
>  		if (active) {
> -			unsigned int clamped_x, clamped_y;
> +			int clamped_x, clamped_y;
>  			/*
>  			 * The touchpad might report invalid data, so we clamp
>  			 * the resulting values so that we do not confuse
>  			 * userspace.
>  			 */
> -			clamped_x = clamp(finger->x, 0U, priv->x_max);
> -			clamped_y = clamp(finger->y, 0U, priv->y_max);
> +			clamped_x = clamp(finger->x, 0, priv->x_max);
> +			clamped_y = clamp(finger->y, 0, priv->y_max);
>  			input_report_abs(dev, ABS_MT_POSITION_X, clamped_x);
>  			input_report_abs(dev, ABS_MT_POSITION_Y,
>  					 priv->y_max - clamped_y);