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

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

[Griffin Kroah-Hartman]

Add size checks for the "len" variable in xpadone_process_packet().
This prevents out-of-bounds accesses to the "data" buffer, as "len"
comes directly from the hardware.

Assisted-by: gkh_clanker_t1000
Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
---
 drivers/input/joystick/xpad.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index feb8f368f834..abda6b5c7c95 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1035,8 +1035,14 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
 	struct input_dev *dev = xpad->dev;
 	bool do_sync = false;
 
+	if (len < 2)
+		return;
+	len = min(len, XPAD_PKT_LEN);
+
 	/* the xbox button has its own special report */
 	if (data[0] == GIP_CMD_VIRTUAL_KEY) {
+		if (len < 5)
+			return;
 		/*
 		 * The Xbox One S controller requires these reports to be
 		 * acked otherwise it continues sending them forever and
@@ -1052,6 +1058,8 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
 	} else if (data[0] == GIP_CMD_FIRMWARE) {
 		/* Some packet formats force us to use this separate to poll paddle inputs */
 		if (xpad->packet_type == PKT_XBE2_FW_5_11) {
+			if (len < 20)
+				return;
 			/* Mute paddles if controller is in a custom profile slot
 			 * Checked by looking at the active profile slot to
 			 * verify it's the default slot
@@ -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;
 			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;
 				/* Mute paddles if controller has a custom mapping applied.
 				 * Checked by comparing the current mapping
 				 * config against the factory mapping config
@@ -1165,6 +1180,8 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
 				input_report_key(dev, BTN_GRIPL, data[32] & BIT(0));
 				input_report_key(dev, BTN_GRIPL2, data[32] & BIT(2));
 			} else if (xpad->packet_type == PKT_XBE2_FW_OLD) {
+				if (len < 20)
+					return;
 				/* Mute paddles if controller has a custom mapping applied.
 				 * Checked by comparing the current mapping
 				 * config against the factory mapping config
@@ -1178,6 +1195,8 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
 				input_report_key(dev, BTN_GRIPL, data[18] & BIT(2));
 				input_report_key(dev, BTN_GRIPL2, data[18] & BIT(3));
 			} else if (xpad->packet_type == PKT_XBE2_FW_5_EARLY) {
+				if (len < 24)
+					return;
 				/* Mute paddles if controller has a custom mapping applied.
 				 * Checked by comparing the current mapping
 				 * config against the factory mapping config

---
base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
change-id: 20260727-xpadone_length_checks-08120b97764f

Best regards,
--  
Griffin Kroah-Hartman <griffin@kroah.com>

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

[sashiko-bot]

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

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/20260727-xpadone_length_checks-v1-1-19aa9331e82d@kroah.com?part=1