Re: [PATCH] Input: focaltech - fix array out-of-bounds in focaltech_process_rel_packet

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Medium] Unsigned coordinate wrapping causes the cursor to incorrectly jump to the opposite edge when processing relative movements.
--

commit 7c58aa99b660b4d96ec349d279fd307555360fd9
Author: Richard Davies <richard@arachsys.com>

Input: focaltech - fix array out-of-bounds in focaltech_process_rel_packet

Make finger2 (and also finger1) unsigned, so that if the finger index in
the packet is 0 then subtracting 1 creates an array index which overflows
above the existing check for FOC_MAX_FINGERS, as the existing comment says
it should, instead of writing to state->fingers[-1].

> diff --git a/drivers/input/mouse/focaltech.c b/drivers/input/mouse/focaltech.c
> index 43f9939b7c63c..d3ad4af5aa09d 100644
> --- a/drivers/input/mouse/focaltech.c
> +++ b/drivers/input/mouse/focaltech.c
> @@ -197,7 +197,7 @@ static void focaltech_process_rel_packet(struct psmouse *psmouse,
>  {
>  	struct focaltech_data *priv = psmouse->private;
>  	struct focaltech_hw_state *state = &priv->state;
> -	int finger1, finger2;
> +	unsigned int finger1, finger2;
>  
>  	state->pressed = packet[0] >> 7;
>  	finger1 = ((packet[0] >> 4) & 0x7) - 1;

[Severity: Medium]
This isn't a bug introduced by this patch, but does the unsigned type of the
finger coordinates cause them to incorrectly wrap around when applying
relative movements?

When processing relative packets:

drivers/input/mouse/focaltech.c:focaltech_process_rel_packet() {
    ...
		state->fingers[finger1].x += (s8)packet[1];
		state->fingers[finger1].y += (s8)packet[2];
    ...
}

Because x and y are defined as unsigned int, subtracting a value that drops
the coordinate below 0 causes an integer underflow, wrapping it to a large
positive number.

Then, when clamping the coordinates:

drivers/input/mouse/focaltech.c:focaltech_report_state() {
    ...
			clamped_x = clamp(finger->x, 0U, priv->x_max);
			clamped_y = clamp(finger->y, 0U, priv->y_max);
    ...
}

The underflowed unsigned value is incorrectly clamped to the maximum bound
instead of 0. This could cause the user's cursor to jump erratically to the
opposite edge of the screen when dragging a finger near the left or bottom
edge of the touchpad.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701190932.14960-1-richard@arachsys.com?part=1