Re: [PATCH 1/3] Input: xpad - add safer data access framework

[Griffin Kroah-Hartman]

Hi Dmitry,

On 8/3/26 6:23 PM, Dmitry Torokhov wrote:
> Hi Griffin,
>
> On Mon, Aug 03, 2026 at 05:07:24PM +0200, Griffin Kroah-Hartman wrote:
> > USB xpad devices could send short messages which would cause reads and
> > writes outside of the data buffer.
> >
> > Fix this by adding the safe_data struct and the sdata_check() function when
> > accessing packet data for input events, and add the usage of this to
> > xpadone_process_packet(), which was vulnerable to OOB reads/writes.
> >
> > Suggested-by: Ingo Molnar <mingo@xxxxxxxxxx>
> > Suggested-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Griffin Kroah-Hartman <griffin@xxxxxxxxx>
> > ---
> >   drivers/input/joystick/xpad.c | 115 ++++++++++++++++++++++++++----------------
> >   1 file changed, 71 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> > index feb8f368f834..c516860711a8 100644
> > --- a/drivers/input/joystick/xpad.c
> > +++ b/drivers/input/joystick/xpad.c
> > @@ -780,6 +780,24 @@ struct usb_xpad {
> >   	bool delayed_init_done;
> >   };
> >   
> > +struct safe_data {
> > +	unsigned char *data;
> > +	u32 len;
> > +};
> > +
> > +/*
> > + * Safe Data Check
> > + *
> > + * Returns the correct data when inside the array's bounds,
> > + * returns 0 when accessing an out-of-bounds index.
> > + */
> > +static u8 sdata_check(struct safe_data *sdata, int idx)
> > +{
> > +	if (idx >= sdata->len)
> > +		return 0;
> > +	return sdata->data[idx];
> > +}
> I'd rather we had explicit length checks for various packets and skipped
> the processing if the packet is short instead of making large number of
> what can be considered repeated checks.

Sure thing, I can instead replicate something similar to my original 
patch here:

https://lore.kernel.org/all/20260727-xpadone_length_checks-v1-1-19aa9331e82d@xxxxxxxxx/

Ingo had suggested this method instead.


Thanks,

Griffin