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

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Mon, Aug 03, 2026 at 01:29:32PM +0000, Richard Davies wrote:
> 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.
> 
> I agree this is theoretically possible. For what it's worth, I haven't
> actually experienced the cursor jumping to the opposite edge of the screen
> on my laptop. That might mean the relative packets are actually safe, e.g.
> generated internally from an absolute position.
> 
> > 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@kernel.org
> > Assisted-by: Antigravity:gemini-3.6-flash
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > 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);
> 
> If it's allowed and race-free to change finger->{x,y} in
> focaltech_report_state, then you might fix the Sashiko [High] issue on this
> patch by just doing:
> 
> finger->x = clamp(finger->x, 0, priv->x_max);
> finger->y = clamp(finger->y, 0, priv->y_max);
> 
> not having the clamped_{x,y} variables at all, and updating the two
> input_report_abs lines below to use finger->{x,y}

Good idea, thank you. I'll update and apply.

Thanks.

-- 
Dmitry