[PATCH 1/3] Input: xpad - add safer data access framework
From: Griffin Kroah-Hartman <hidden>
Date: 2026-08-03 15:07:32
Also in:
lkml
Subsystem:
input (keyboard, mouse, joystick, touchscreen) drivers, the rest · Maintainers:
Dmitry Torokhov, Linus Torvalds
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 <redacted> --- 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]; +} + static int xpad_init_input(struct usb_xpad *xpad); static void xpad_deinit_input(struct usb_xpad *xpad); static int xpad_start_input(struct usb_xpad *xpad);
@@ -1033,41 +1051,45 @@ static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned cha static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data, u32 len) { struct input_dev *dev = xpad->dev; + struct safe_data sdata; bool do_sync = false; + sdata.data = data; + sdata.len = len; + /* the xbox button has its own special report */ - if (data[0] == GIP_CMD_VIRTUAL_KEY) { + if (sdata_check(&sdata, 0) == GIP_CMD_VIRTUAL_KEY) { /* * The Xbox One S controller requires these reports to be * acked otherwise it continues sending them forever and * won't report further mode button events. */ - if (data[1] == (GIP_OPT_ACK | GIP_OPT_INTERNAL)) - xpadone_ack_mode_report(xpad, data[2]); + if (sdata_check(&sdata, 1) == (GIP_OPT_ACK | GIP_OPT_INTERNAL)) + xpadone_ack_mode_report(xpad, sdata_check(&sdata, 2)); - input_report_key(dev, BTN_MODE, data[4] & GENMASK(1, 0)); + input_report_key(dev, BTN_MODE, sdata_check(&sdata, 4) & GENMASK(1, 0)); input_sync(dev); do_sync = true; - } else if (data[0] == GIP_CMD_FIRMWARE) { + } else if (sdata_check(&sdata, 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) { /* 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 */ - if (data[19] != 0) + if (sdata_check(&sdata, 19) != 0) data[18] = 0; /* Elite Series 2 split packet paddle bits */ - input_report_key(dev, BTN_GRIPR, data[18] & BIT(0)); - input_report_key(dev, BTN_GRIPR2, data[18] & BIT(1)); - input_report_key(dev, BTN_GRIPL, data[18] & BIT(2)); - input_report_key(dev, BTN_GRIPL2, data[18] & BIT(3)); + input_report_key(dev, BTN_GRIPR, sdata_check(&sdata, 18) & BIT(0)); + input_report_key(dev, BTN_GRIPR2, sdata_check(&sdata, 18) & BIT(1)); + input_report_key(dev, BTN_GRIPL, sdata_check(&sdata, 18) & BIT(2)); + input_report_key(dev, BTN_GRIPL2, sdata_check(&sdata, 18) & BIT(3)); do_sync = true; } - } else if (data[0] == GIP_CMD_ANNOUNCE) { + } else if (sdata_check(&sdata, 0) == GIP_CMD_ANNOUNCE) { int error; if (xpad->delay_init && !xpad->delayed_init_done) {
@@ -1078,44 +1100,49 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char "unable to start delayed input: %d\n", error); } - } else if (data[0] == GIP_CMD_INPUT) { /* The main valid packet type for inputs */ + } else if (sdata_check(&sdata, 0) == GIP_CMD_INPUT) { + /* The main valid packet type for inputs */ + /* menu/view buttons */ - input_report_key(dev, BTN_START, data[4] & BIT(2)); - input_report_key(dev, BTN_SELECT, data[4] & BIT(3)); + input_report_key(dev, BTN_START, sdata_check(&sdata, 4) & BIT(2)); + input_report_key(dev, BTN_SELECT, sdata_check(&sdata, 4) & BIT(3)); if (xpad->mapping & MAP_SHARE_BUTTON) { u32 offset = (xpad->mapping & MAP_SHARE_OFFSET) ? 26 : 18; if (len >= offset) - input_report_key(dev, KEY_RECORD, data[len - offset] & BIT(0)); + input_report_key(dev, KEY_RECORD, + sdata_check(&sdata, len - offset) & BIT(0)); } /* buttons A,B,X,Y */ - input_report_key(dev, BTN_A, data[4] & BIT(4)); - input_report_key(dev, BTN_B, data[4] & BIT(5)); - input_report_key(dev, BTN_X, data[4] & BIT(6)); - input_report_key(dev, BTN_Y, data[4] & BIT(7)); + input_report_key(dev, BTN_A, sdata_check(&sdata, 4) & BIT(4)); + input_report_key(dev, BTN_B, sdata_check(&sdata, 4) & BIT(5)); + input_report_key(dev, BTN_X, sdata_check(&sdata, 4) & BIT(6)); + input_report_key(dev, BTN_Y, sdata_check(&sdata, 4) & BIT(7)); /* digital pad */ if (xpad->mapping & MAP_DPAD_TO_BUTTONS) { /* dpad as buttons (left, right, up, down) */ - input_report_key(dev, BTN_DPAD_LEFT, data[5] & BIT(2)); - input_report_key(dev, BTN_DPAD_RIGHT, data[5] & BIT(3)); - input_report_key(dev, BTN_DPAD_UP, data[5] & BIT(0)); - input_report_key(dev, BTN_DPAD_DOWN, data[5] & BIT(1)); + input_report_key(dev, BTN_DPAD_LEFT, sdata_check(&sdata, 5) & BIT(2)); + input_report_key(dev, BTN_DPAD_RIGHT, sdata_check(&sdata, 5) & BIT(3)); + input_report_key(dev, BTN_DPAD_UP, sdata_check(&sdata, 5) & BIT(0)); + input_report_key(dev, BTN_DPAD_DOWN, sdata_check(&sdata, 5) & BIT(1)); } else { input_report_abs(dev, ABS_HAT0X, - !!(data[5] & 0x08) - !!(data[5] & 0x04)); + !!(sdata_check(&sdata, 5) & 0x08) - + !!(sdata_check(&sdata, 5) & 0x04)); input_report_abs(dev, ABS_HAT0Y, - !!(data[5] & 0x02) - !!(data[5] & 0x01)); + !!(sdata_check(&sdata, 5) & 0x02) - + !!(sdata_check(&sdata, 5) & 0x01)); } /* TL/TR */ - input_report_key(dev, BTN_TL, data[5] & BIT(4)); - input_report_key(dev, BTN_TR, data[5] & BIT(5)); + input_report_key(dev, BTN_TL, sdata_check(&sdata, 5) & BIT(4)); + input_report_key(dev, BTN_TR, sdata_check(&sdata, 5) & BIT(5)); /* stick press left/right */ - input_report_key(dev, BTN_THUMBL, data[5] & BIT(6)); - input_report_key(dev, BTN_THUMBR, data[5] & BIT(7)); + input_report_key(dev, BTN_THUMBL, sdata_check(&sdata, 5) & BIT(6)); + input_report_key(dev, BTN_THUMBR, sdata_check(&sdata, 5) & BIT(7)); if (!(xpad->mapping & MAP_STICKS_TO_NULL)) { /* left stick */
@@ -1146,7 +1173,7 @@ 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) - input_report_abs(dev, ABS_PROFILE, data[34]); + input_report_abs(dev, ABS_PROFILE, sdata_check(&sdata, 34)); /* paddle handling */ /* based on SDL's SDL_hidapi_xboxone.c */
@@ -1160,38 +1187,38 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char data[32] = 0; /* OG Elite Series Controller paddle bits */ - input_report_key(dev, BTN_GRIPR, data[32] & BIT(1)); - input_report_key(dev, BTN_GRIPR2, data[32] & BIT(3)); - input_report_key(dev, BTN_GRIPL, data[32] & BIT(0)); - input_report_key(dev, BTN_GRIPL2, data[32] & BIT(2)); + input_report_key(dev, BTN_GRIPR, sdata_check(&sdata, 32) & BIT(1)); + input_report_key(dev, BTN_GRIPR2, sdata_check(&sdata, 32) & BIT(3)); + input_report_key(dev, BTN_GRIPL, sdata_check(&sdata, 32) & BIT(0)); + input_report_key(dev, BTN_GRIPL2, sdata_check(&sdata, 32) & BIT(2)); } else if (xpad->packet_type == PKT_XBE2_FW_OLD) { /* Mute paddles if controller has a custom mapping applied. * Checked by comparing the current mapping * config against the factory mapping config */ - if (data[19] != 0) + if (sdata_check(&sdata, 19) != 0) data[18] = 0; /* Elite Series 2 4.x firmware paddle bits */ - input_report_key(dev, BTN_GRIPR, data[18] & BIT(0)); - input_report_key(dev, BTN_GRIPR2, data[18] & BIT(1)); - input_report_key(dev, BTN_GRIPL, data[18] & BIT(2)); - input_report_key(dev, BTN_GRIPL2, data[18] & BIT(3)); + input_report_key(dev, BTN_GRIPR, sdata_check(&sdata, 18) & BIT(0)); + input_report_key(dev, BTN_GRIPR2, sdata_check(&sdata, 18) & BIT(1)); + input_report_key(dev, BTN_GRIPL, sdata_check(&sdata, 18) & BIT(2)); + input_report_key(dev, BTN_GRIPL2, sdata_check(&sdata, 18) & BIT(3)); } else if (xpad->packet_type == PKT_XBE2_FW_5_EARLY) { /* Mute paddles if controller has a custom mapping applied. * Checked by comparing the current mapping * config against the factory mapping config */ - if (data[23] != 0) + if (sdata_check(&sdata, 23) != 0) data[22] = 0; /* Elite Series 2 5.x firmware paddle bits * (before the packet was split) */ - input_report_key(dev, BTN_GRIPR, data[22] & BIT(0)); - input_report_key(dev, BTN_GRIPR2, data[22] & BIT(1)); - input_report_key(dev, BTN_GRIPL, data[22] & BIT(2)); - input_report_key(dev, BTN_GRIPL2, data[22] & BIT(3)); + input_report_key(dev, BTN_GRIPR, sdata_check(&sdata, 22) & BIT(0)); + input_report_key(dev, BTN_GRIPR2, sdata_check(&sdata, 22) & BIT(1)); + input_report_key(dev, BTN_GRIPL, sdata_check(&sdata, 22) & BIT(2)); + input_report_key(dev, BTN_GRIPL2, sdata_check(&sdata, 22) & BIT(3)); } }
--
2.55.0