From: Griffin Kroah-Hartman <griffin@kroah.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Griffin Kroah-Hartman <griffin@kroah.com>,
Ingo Molnar <mingo@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: [PATCH 1/3] Input: xpad - add safer data access framework
Date: Mon, 03 Aug 2026 17:07:24 +0200 [thread overview]
Message-ID: <20260803-xpadone_packet_fix-v1-1-280da203f15c@kroah.com> (raw)
In-Reply-To: <20260803-xpadone_packet_fix-v1-0-280da203f15c@kroah.com>
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];
+}
+
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
next prev parent reply other threads:[~2026-08-03 15:07 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-03 15:07 [PATCH 0/3] XPAD safety strengthening Griffin Kroah-Hartman
2026-08-03 15:07 ` Griffin Kroah-Hartman [this message]
2026-08-03 15:27 ` [PATCH 1/3] Input: xpad - add safer data access framework sashiko-bot
2026-08-03 16:23 ` Dmitry Torokhov
2026-08-04 8:02 ` Griffin Kroah-Hartman
2026-08-03 15:07 ` [PATCH 2/3] Input: xpad - add sdata_check() to xpad controllers Griffin Kroah-Hartman
2026-08-03 15:46 ` sashiko-bot
2026-08-03 15:07 ` [PATCH 3/3] Input: xpad - add sdata_check() to xpad360 branches Griffin Kroah-Hartman
2026-08-03 16:05 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260803-xpadone_packet_fix-v1-1-280da203f15c@kroah.com \
--to=griffin@kroah.com \
--cc=dmitry.torokhov@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox