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

[Griffin Kroah-Hartman <griffin@kroah.com>]

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@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.

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@kroah.com/

Ingo had suggested this method instead.


Thanks,

Griffin