Re: [PATCH] Input: xpad - add out-of-bounds checks for xpadone

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Early returns during packet processing bypass `input_sync()`, leaving previously reported events unsynchronized.
--

--- Patch [1]: [PATCH] Input: xpad - add out-of-bounds checks for xpadone ---
commit 8c34b8de402ad35f3c36f154396b4b591175f380
Author: Griffin Kroah-Hartman <[email protected]>

Input: xpad - add out-of-bounds checks for xpadone
    
Add size checks for the len variable in xpadone_process_packet() to
prevent out-of-bounds accesses to the data buffer.

> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -1079,6 +1087,8 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
>  					 error);
>  		}
>  	} else if (data[0] == GIP_CMD_INPUT) { /* The main valid packet type for inputs */
> +		if (len < 18)
> +			return;
>  		/* menu/view buttons */
>  		input_report_key(dev, BTN_START,  data[4] & BIT(2));
>  		input_report_key(dev, BTN_SELECT, data[4] & BIT(3));

[ ... ]

> @@ -1145,13 +1155,18 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
>  		}
>  
>  		/* Profile button has a value of 0-3, so it is reported as an axis */
> -		if (xpad->mapping & MAP_PROFILE_BUTTON)
> +		if (xpad->mapping & MAP_PROFILE_BUTTON) {
> +			if (len < 35)
> +				return;

[Severity: High]
By returning early here, do we bypass the synchronization of the standard 
buttons and axes that were already reported earlier in this function?

If the controller sends a valid base GIP_CMD_INPUT packet (>= 18 bytes) but 
shorter than 35 bytes, this early return skips the do_sync = true assignment 
and the final input_sync() call at the end of xpadone_process_packet(). 

Would this result in standard inputs becoming unresponsive since the events are 
buffered by the input core but never delivered to userspace?

>  			input_report_abs(dev, ABS_PROFILE, data[34]);
> +		}
>  
>  		/* paddle handling */
>  		/* based on SDL's SDL_hidapi_xboxone.c */
>  		if (xpad->mapping & MAP_PADDLES) {
>  			if (xpad->packet_type == PKT_XBE1) {
> +				if (len < 33)
> +					return;

[Severity: High]
This early return, as well as the ones below for PKT_XBE2_FW_OLD and
PKT_XBE2_FW_5_EARLY, appear to have the same issue. Do they also skip 
input_sync() for the standard inputs?

>  				/* Mute paddles if controller has a custom mapping applied.
>  				 * Checked by comparing the current mapping
>  				 * config against the factory mapping config

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1