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

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

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@kernel.org>
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
> ---
>  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.

Thanks.

-- 
Dmitry