Re: [PATCH] Input: wacom_w8001 - validate index before storing data byte

[Muhammad Bilal]

Hi Dmitry,

Thanks for the review. You are right. idx advances by one per call and
the W8001_PKTLEN_TOUCH2FG - 1 case resets it unconditionally, so it
always stays in [0, 12] against a 13 byte buffer. I rechecked against
current master and w8001_interrupt() is the only place that writes
idx, so the store cannot go out of bounds. The commit message was
wrong to say nothing stops idx from growing.

Please drop this patch.

Thanks,
Muhammad

On Sun, Sep 20, 2026 at 9:02 AM Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
>
> Hi Muhammad,
>
> On Sun, Sep 20, 2026 at 12:21:53AM +0500, Muhammad Bilal wrote:
> > w8001_interrupt() stores every incoming byte at w8001->data[w8001->idx]
> > before the following switch on w8001->idx++ has a chance to detect an
> > invalid packet and reset idx. The switch only resets idx for the
> > specific packet lengths it recognizes; once idx has advanced past all
> > of those (W8001_PKTLEN_TOUCH2FG - 1 at most), any further byte falls
> > into default, where idx is only reset for pen-only devices without a
> > touch_dev (the ThinkPad X60 workaround). A touch-capable device fed a
> > malformed or overlong packet therefore has nothing to stop idx from
> > growing without bound, and w8001->data[w8001->idx] = data runs past
> > the end of the W8001_MAX_LENGTH-sized array.
>
> This analysis does not match the code.
>
> w8001->idx starts at 0 and is incremented by 1 on each invocation via
> switch (w8001->idx++). To advance past W8001_PKTLEN_TOUCH2FG - 1 (12),
> w8001->idx would first have to pass through 12, which matches:
>
>         /* 2 finger touch packet */
>         case W8001_PKTLEN_TOUCH2FG - 1:
>                 w8001->idx = 0;
>                 parse_multi_touch(w8001);
>                 break;
>
> Unlike the shorter packet length cases, this case has no conditional
> break and unconditionally resets w8001->idx to 0. Since W8001_MAX_LENGTH
> is 13 (matching W8001_PKTLEN_TOUCH2FG), w8001->idx is always in the
> [0, 12] range when entering w8001_interrupt().
>
> As a result, w8001->idx >= W8001_MAX_LENGTH is unreachable and the array
> store cannot go out of bounds.
>
> Thanks.
>
> --
> Dmitry