Linux kernel -stable discussions
 help / color / mirror / Atom feed
* [PATCH 1/3] Input: atmel_mxt_ts - fix boundary check in mxt_prepare_cfg_mem
@ 2026-05-04 18:54 Dmitry Torokhov
  2026-05-05  9:08 ` Ricardo Ribalda
  0 siblings, 1 reply; 3+ messages in thread
From: Dmitry Torokhov @ 2026-05-04 18:54 UTC (permalink / raw)
  To: Nick Dyer, linux-input; +Cc: Ricardo Ribalda, linux-kernel, stable

When a configuration file provides an object size that is larger than the
driver's known mxt_obj_size(object), the driver intends to discard the
extra bytes.

The loop iterates using for (i = 0; i < size; i++). Inside the loop, the
condition to skip processing extra bytes is:

    if (i > mxt_obj_size(object))
        continue;

Since i is a 0-based index, the valid indices for the object are 0 through
mxt_obj_size(object) - 1.

When i == mxt_obj_size(object), the condition evaluates to false, and the
code processes the byte instead of discarding it.

This causes the code to calculate byte_offset = reg + i - cfg->start_ofs
and writes the byte there, overwriting exactly one byte of the adjacent
instance or object.

Update the boundary check to skip extra bytes correctly by using >=.

Fixes: 50a77c658b80 ("Input: atmel_mxt_ts - download device config using firmware loader")
Cc: stable@vger.kernel.org
Assisted-by: Gemini:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index d62bf2c95578..28b2bd889c70 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1503,7 +1503,7 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data, struct mxt_cfg *cfg)
 			}
 			cfg->raw_pos += offset;
 
-			if (i > mxt_obj_size(object))
+			if (i >= mxt_obj_size(object))
 				continue;
 
 			byte_offset = reg + i - cfg->start_ofs;
-- 
2.54.0.545.g6539524ca2-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/3] Input: atmel_mxt_ts - fix boundary check in mxt_prepare_cfg_mem
  2026-05-04 18:54 [PATCH 1/3] Input: atmel_mxt_ts - fix boundary check in mxt_prepare_cfg_mem Dmitry Torokhov
@ 2026-05-05  9:08 ` Ricardo Ribalda
  2026-05-05 15:03   ` Dmitry Torokhov
  0 siblings, 1 reply; 3+ messages in thread
From: Ricardo Ribalda @ 2026-05-05  9:08 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Nick Dyer, linux-input, linux-kernel, stable

HI Dmitry

FWIW this patch looks correct to me...

Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>

But there are a couple of things that look weird.

1) The patch line (1503) does not seem to match your tree
https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/tree/drivers/input/touchscreen/atmel_mxt_ts.c#n1503

2) The sscanf just before this check has two conversions (val and
offset), but you only check for ret != 1. Should't it be ret !=2? or I
am missing something?

On Mon, 4 May 2026 at 20:54, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>
> When a configuration file provides an object size that is larger than the
> driver's known mxt_obj_size(object), the driver intends to discard the
> extra bytes.
>
> The loop iterates using for (i = 0; i < size; i++). Inside the loop, the
> condition to skip processing extra bytes is:
>
>     if (i > mxt_obj_size(object))
>         continue;
>
> Since i is a 0-based index, the valid indices for the object are 0 through
> mxt_obj_size(object) - 1.
>
> When i == mxt_obj_size(object), the condition evaluates to false, and the
> code processes the byte instead of discarding it.
>
> This causes the code to calculate byte_offset = reg + i - cfg->start_ofs
> and writes the byte there, overwriting exactly one byte of the adjacent
> instance or object.
>
> Update the boundary check to skip extra bytes correctly by using >=.
>
> Fixes: 50a77c658b80 ("Input: atmel_mxt_ts - download device config using firmware loader")
> Cc: stable@vger.kernel.org
> Assisted-by: Gemini:gemini-3.1-pro
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index d62bf2c95578..28b2bd889c70 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -1503,7 +1503,7 @@ static int mxt_prepare_cfg_mem(struct mxt_data *data, struct mxt_cfg *cfg)
>                         }
>                         cfg->raw_pos += offset;
>
> -                       if (i > mxt_obj_size(object))
> +                       if (i >= mxt_obj_size(object))
>                                 continue;
>
>                         byte_offset = reg + i - cfg->start_ofs;
> --
> 2.54.0.545.g6539524ca2-goog
>


-- 
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/3] Input: atmel_mxt_ts - fix boundary check in mxt_prepare_cfg_mem
  2026-05-05  9:08 ` Ricardo Ribalda
@ 2026-05-05 15:03   ` Dmitry Torokhov
  0 siblings, 0 replies; 3+ messages in thread
From: Dmitry Torokhov @ 2026-05-05 15:03 UTC (permalink / raw)
  To: Ricardo Ribalda; +Cc: Nick Dyer, linux-input, linux-kernel, stable

Hi Ricardo,

On Tue, May 05, 2026 at 11:08:15AM +0200, Ricardo Ribalda wrote:
> HI Dmitry
> 
> FWIW this patch looks correct to me...

Thank you for looking this over.

> 
> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> 
> But there are a couple of things that look weird.
> 
> 1) The patch line (1503) does not seem to match your tree
> https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/tree/drivers/input/touchscreen/atmel_mxt_ts.c#n1503

Yeah, I have an unrelated path in my queue that affects line offsets,

> 
> 2) The sscanf just before this check has two conversions (val and
> offset), but you only check for ret != 1. Should't it be ret !=2? or I
> am missing something?

"%n" format specifier does not increment number of successfully parsed
elements returned by sscanf(). It kind of makes sense although may look
surprising.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-05 15:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-04 18:54 [PATCH 1/3] Input: atmel_mxt_ts - fix boundary check in mxt_prepare_cfg_mem Dmitry Torokhov
2026-05-05  9:08 ` Ricardo Ribalda
2026-05-05 15:03   ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox