Thread (2 messages) 2 messages, 2 authors, 1d ago

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

From: sashiko-bot@kernel.org
Date: 2026-07-27 16:19:03

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 [off-list ref]

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.
quoted hunk ↗ jump to hunk
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));
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help