Re: [PATCH 1/6] Input: mms114 - fix touch indexing for MMS134S and MMS136

[Dmitry Torokhov]

Hi Bryam,

On Tue, Jun 16, 2026 at 07:05:34AM +0000, Bryam Vargas wrote:
> Hi Dmitry,
> 
> The indexing fix looks correct -- deriving the byte offset from event_size
> instead of leaning on sizeof(struct mms114_touch) is the right call, and the
> cast is fine since the struct is __packed (no alignment issue at the 6-byte
> stride, and the consumers only touch bytes 0..5).
> 
> Two things that might be worth folding into the series:
> 
> 1) Fixes: tag.  The 6-byte event path for MMS136 -- and therefore this
>    stride-vs-index mismatch -- predates ab108678195f.  It was introduced in
> 
>        53fefdd1d3a3 ("Input: mms114 - support MMS136")
> 
>    which added MMS136_EVENT_SIZE and the "packet_size / MMS136_EVENT_SIZE"
>    branch while the loop already indexed with the 8-byte struct stride;
>    ab108678195f ("support MMS134S") only extended that branch to MMS134S.
>    So MMS136 hardware has mis-parsed multi-touch since v5.13.  Tagging
> 
>        Fixes: 53fefdd1d3a3 ("Input: mms114 - support MMS136")
> 
>    (in addition to, or instead of, ab108678195f) lets stable pick it up for
>    the MMS136 range as well.

Thanks, I'll update the tag.

> 
> 2) Unbounded packet_size.  Since 1/6 already rewrites this handler: packet_size
>    comes straight from the device's PACKET_SIZE register (a single byte, so
>    1..255 after the "<= 0" guard) and is then used unclamped both as the read
>    length
> 
>        __mms114_read_reg(data, MMS114_INFORMATION, packet_size, (u8 *)touch);
> 
>    into the 80-byte touch[MMS114_MAX_TOUCH] stack buffer, and as the divisor
>    for touch_size -- which is never bounded by MMS114_MAX_TOUCH.  A device that
>    reports packet_size > 80 overflows the stack buffer on the read, and even
>    with the indexing fix the loop still walks past it (touch_size > 10).  A
>    one-liner before the division closes both:
> 
>        if (packet_size <= 0)
>                goto out;
>    +   packet_size = min_t(int, packet_size, sizeof(touch));
> 
>    This one is pre-existing -- the unbounded read goes back to the original
>    driver -- so it is really a separate patch in the series rather than part
>    of the indexing fix, but it seemed worth raising while the code is in
>    flight.

This is fixed by the patch you sent earlier, right? It's been applied so
I did not send it out again with the series.

Thank you for looking at this, please do not hesitate to add your
Reviewed-by to any patches that you are satisfied with.

Thanks.

-- 
Dmitry