* [PATCH 1/3] Input: xpad - add safer data access framework
2026-08-03 15:07 [PATCH 0/3] XPAD safety strengthening Griffin Kroah-Hartman
@ 2026-08-03 15:07 ` Griffin Kroah-Hartman
2026-08-03 16:23 ` Dmitry Torokhov
2026-08-03 15:07 ` [PATCH 2/3] Input: xpad - add sdata_check() to xpad controllers Griffin Kroah-Hartman
2026-08-03 15:07 ` [PATCH 3/3] Input: xpad - add sdata_check() to xpad360 branches Griffin Kroah-Hartman
2 siblings, 1 reply; 6+ messages in thread
From: Griffin Kroah-Hartman @ 2026-08-03 15:07 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, linux-kernel, Griffin Kroah-Hartman, Ingo Molnar,
Greg Kroah-Hartman
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/3] Input: xpad - add safer data access framework
2026-08-03 15:07 ` [PATCH 1/3] Input: xpad - add safer data access framework Griffin Kroah-Hartman
@ 2026-08-03 16:23 ` Dmitry Torokhov
2026-08-04 8:02 ` Griffin Kroah-Hartman
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2026-08-03 16:23 UTC (permalink / raw)
To: Griffin Kroah-Hartman
Cc: linux-input, linux-kernel, Ingo Molnar, Greg Kroah-Hartman
Hi Griffin,
On Mon, Aug 03, 2026 at 05:07:24PM +0200, Griffin Kroah-Hartman wrote:
> 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];
> +}
I'd rather we had explicit length checks for various packets and skipped
the processing if the packet is short instead of making large number of
what can be considered repeated checks.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/3] Input: xpad - add safer data access framework
2026-08-03 16:23 ` Dmitry Torokhov
@ 2026-08-04 8:02 ` Griffin Kroah-Hartman
0 siblings, 0 replies; 6+ messages in thread
From: Griffin Kroah-Hartman @ 2026-08-04 8:02 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, linux-kernel, Ingo Molnar, Greg Kroah-Hartman
Hi Dmitry,
On 8/3/26 6:23 PM, Dmitry Torokhov wrote:
> Hi Griffin,
>
> On Mon, Aug 03, 2026 at 05:07:24PM +0200, Griffin Kroah-Hartman wrote:
>> 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];
>> +}
> I'd rather we had explicit length checks for various packets and skipped
> the processing if the packet is short instead of making large number of
> what can be considered repeated checks.
Sure thing, I can instead replicate something similar to my original
patch here:
https://lore.kernel.org/all/20260727-xpadone_length_checks-v1-1-19aa9331e82d@kroah.com/
Ingo had suggested this method instead.
Thanks,
Griffin
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] Input: xpad - add sdata_check() to xpad controllers
2026-08-03 15:07 [PATCH 0/3] XPAD safety strengthening Griffin Kroah-Hartman
2026-08-03 15:07 ` [PATCH 1/3] Input: xpad - add safer data access framework Griffin Kroah-Hartman
@ 2026-08-03 15:07 ` Griffin Kroah-Hartman
2026-08-03 15:07 ` [PATCH 3/3] Input: xpad - add sdata_check() to xpad360 branches Griffin Kroah-Hartman
2 siblings, 0 replies; 6+ messages in thread
From: Griffin Kroah-Hartman @ 2026-08-03 15:07 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, linux-kernel, Griffin Kroah-Hartman, Ingo Molnar,
Greg Kroah-Hartman
Add the sdata_check() safety wrapper to the xpad_process_packet()
branch. Which should include the original Xbox Controller.
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 | 50 ++++++++++++++++++++++++-------------------
1 file changed, 28 insertions(+), 22 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index c516860711a8..319a4c4a695f 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -813,9 +813,13 @@ static void xpad360w_poweroff_controller(struct usb_xpad *xpad);
* The used report descriptor was taken from ITO Takayuki's website:
* http://euc.jp/periphs/xbox-controller.ja.html
*/
-static void xpad_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data)
+static void xpad_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data, u32 len)
{
struct input_dev *dev = xpad->dev;
+ struct safe_data sdata;
+
+ sdata.data = data;
+ sdata.len = len;
if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
/* left stick */
@@ -833,42 +837,44 @@ static void xpad_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *d
/* triggers left/right */
if (xpad->mapping & MAP_TRIGGERS_TO_BUTTONS) {
- input_report_key(dev, BTN_TL2, data[10]);
- input_report_key(dev, BTN_TR2, data[11]);
+ input_report_key(dev, BTN_TL2, sdata_check(&sdata, 10));
+ input_report_key(dev, BTN_TR2, sdata_check(&sdata, 11));
} else {
- input_report_abs(dev, ABS_Z, data[10]);
- input_report_abs(dev, ABS_RZ, data[11]);
+ input_report_abs(dev, ABS_Z, sdata_check(&sdata, 10));
+ input_report_abs(dev, ABS_RZ, sdata_check(&sdata, 11));
}
/* digital pad */
if (xpad->mapping & MAP_DPAD_TO_BUTTONS) {
/* dpad as buttons (left, right, up, down) */
- input_report_key(dev, BTN_DPAD_LEFT, data[2] & BIT(2));
- input_report_key(dev, BTN_DPAD_RIGHT, data[2] & BIT(3));
- input_report_key(dev, BTN_DPAD_UP, data[2] & BIT(0));
- input_report_key(dev, BTN_DPAD_DOWN, data[2] & BIT(1));
+ input_report_key(dev, BTN_DPAD_LEFT, sdata_check(&sdata, 2) & BIT(2));
+ input_report_key(dev, BTN_DPAD_RIGHT, sdata_check(&sdata, 2) & BIT(3));
+ input_report_key(dev, BTN_DPAD_UP, sdata_check(&sdata, 2) & BIT(0));
+ input_report_key(dev, BTN_DPAD_DOWN, sdata_check(&sdata, 2) & BIT(1));
} else {
input_report_abs(dev, ABS_HAT0X,
- !!(data[2] & 0x08) - !!(data[2] & 0x04));
+ !!(sdata_check(&sdata, 2) & 0x08) -
+ !!(sdata_check(&sdata, 2) & 0x04));
input_report_abs(dev, ABS_HAT0Y,
- !!(data[2] & 0x02) - !!(data[2] & 0x01));
+ !!(sdata_check(&sdata, 2) & 0x02) -
+ !!(sdata_check(&sdata, 2) & 0x01));
}
/* start/back buttons and stick press left/right */
- input_report_key(dev, BTN_START, data[2] & BIT(4));
- input_report_key(dev, BTN_SELECT, data[2] & BIT(5));
- input_report_key(dev, BTN_THUMBL, data[2] & BIT(6));
- input_report_key(dev, BTN_THUMBR, data[2] & BIT(7));
+ input_report_key(dev, BTN_START, sdata_check(&sdata, 2) & BIT(4));
+ input_report_key(dev, BTN_SELECT, sdata_check(&sdata, 2) & BIT(5));
+ input_report_key(dev, BTN_THUMBL, sdata_check(&sdata, 2) & BIT(6));
+ input_report_key(dev, BTN_THUMBR, sdata_check(&sdata, 2) & BIT(7));
/* "analog" buttons A, B, X, Y */
- input_report_key(dev, BTN_A, data[4]);
- input_report_key(dev, BTN_B, data[5]);
- input_report_key(dev, BTN_X, data[6]);
- input_report_key(dev, BTN_Y, data[7]);
+ input_report_key(dev, BTN_A, sdata_check(&sdata, 4));
+ input_report_key(dev, BTN_B, sdata_check(&sdata, 5));
+ input_report_key(dev, BTN_X, sdata_check(&sdata, 6));
+ input_report_key(dev, BTN_Y, sdata_check(&sdata, 7));
/* "analog" buttons black, white */
- input_report_key(dev, BTN_C, data[8]);
- input_report_key(dev, BTN_Z, data[9]);
+ input_report_key(dev, BTN_C, sdata_check(&sdata, 8));
+ input_report_key(dev, BTN_Z, sdata_check(&sdata, 9));
input_sync(dev);
@@ -1265,7 +1271,7 @@ static void xpad_irq_in(struct urb *urb)
xpadone_process_packet(xpad, 0, xpad->idata, urb->actual_length);
break;
default:
- xpad_process_packet(xpad, 0, xpad->idata);
+ xpad_process_packet(xpad, 0, xpad->idata, urb->actual_length);
}
exit:
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] Input: xpad - add sdata_check() to xpad360 branches
2026-08-03 15:07 [PATCH 0/3] XPAD safety strengthening Griffin Kroah-Hartman
2026-08-03 15:07 ` [PATCH 1/3] Input: xpad - add safer data access framework 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:07 ` Griffin Kroah-Hartman
2 siblings, 0 replies; 6+ messages in thread
From: Griffin Kroah-Hartman @ 2026-08-03 15:07 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, linux-kernel, Griffin Kroah-Hartman, Ingo Molnar,
Greg Kroah-Hartman
Add the sdata_check() safety wrapper to the xpad360_process_packet() and
xpad360w_process_packet() functions, covering the Xbox 360 wired and
wireless controllers.
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 | 75 +++++++++++++++++++++++++------------------
1 file changed, 43 insertions(+), 32 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 319a4c4a695f..304229782e45 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -891,19 +891,24 @@ static void xpad_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *d
*/
static void xpad360_process_packet(struct usb_xpad *xpad, struct input_dev *dev,
- u16 cmd, unsigned char *data)
+ u16 cmd, unsigned char *data, u32 len)
{
+ struct safe_data sdata;
+
+ sdata.data = data;
+ sdata.len = len;
+
/* valid pad data */
- if (data[0] != 0x00)
+ if (sdata_check(&sdata, 0) != 0x00)
return;
/* digital pad */
if (xpad->mapping & MAP_DPAD_TO_BUTTONS) {
/* dpad as buttons (left, right, up, down) */
- input_report_key(dev, BTN_DPAD_LEFT, data[2] & BIT(2));
- input_report_key(dev, BTN_DPAD_RIGHT, data[2] & BIT(3));
- input_report_key(dev, BTN_DPAD_UP, data[2] & BIT(0));
- input_report_key(dev, BTN_DPAD_DOWN, data[2] & BIT(1));
+ input_report_key(dev, BTN_DPAD_LEFT, sdata_check(&sdata, 2) & BIT(2));
+ input_report_key(dev, BTN_DPAD_RIGHT, sdata_check(&sdata, 2) & BIT(3));
+ input_report_key(dev, BTN_DPAD_UP, sdata_check(&sdata, 2) & BIT(0));
+ input_report_key(dev, BTN_DPAD_DOWN, sdata_check(&sdata, 2) & BIT(1));
}
/*
@@ -915,27 +920,29 @@ static void xpad360_process_packet(struct usb_xpad *xpad, struct input_dev *dev,
if (!(xpad->mapping & MAP_DPAD_TO_BUTTONS) ||
xpad->xtype == XTYPE_XBOX360W) {
input_report_abs(dev, ABS_HAT0X,
- !!(data[2] & 0x08) - !!(data[2] & 0x04));
+ !!(sdata_check(&sdata, 2) & 0x08) -
+ !!(sdata_check(&sdata, 2) & 0x04));
input_report_abs(dev, ABS_HAT0Y,
- !!(data[2] & 0x02) - !!(data[2] & 0x01));
+ !!(sdata_check(&sdata, 2) & 0x02) -
+ !!(sdata_check(&sdata, 2) & 0x01));
}
/* start/back buttons */
- input_report_key(dev, BTN_START, data[2] & BIT(4));
- input_report_key(dev, BTN_SELECT, data[2] & BIT(5));
+ input_report_key(dev, BTN_START, sdata_check(&sdata, 2) & BIT(4));
+ input_report_key(dev, BTN_SELECT, sdata_check(&sdata, 2) & BIT(5));
/* stick press left/right */
- input_report_key(dev, BTN_THUMBL, data[2] & BIT(6));
- input_report_key(dev, BTN_THUMBR, data[2] & BIT(7));
+ input_report_key(dev, BTN_THUMBL, sdata_check(&sdata, 2) & BIT(6));
+ input_report_key(dev, BTN_THUMBR, sdata_check(&sdata, 2) & BIT(7));
/* buttons A,B,X,Y,TL,TR and MODE */
- input_report_key(dev, BTN_A, data[3] & BIT(4));
- input_report_key(dev, BTN_B, data[3] & BIT(5));
- input_report_key(dev, BTN_X, data[3] & BIT(6));
- input_report_key(dev, BTN_Y, data[3] & BIT(7));
- input_report_key(dev, BTN_TL, data[3] & BIT(0));
- input_report_key(dev, BTN_TR, data[3] & BIT(1));
- input_report_key(dev, BTN_MODE, data[3] & BIT(2));
+ input_report_key(dev, BTN_A, sdata_check(&sdata, 3) & BIT(4));
+ input_report_key(dev, BTN_B, sdata_check(&sdata, 3) & BIT(5));
+ input_report_key(dev, BTN_X, sdata_check(&sdata, 3) & BIT(6));
+ input_report_key(dev, BTN_Y, sdata_check(&sdata, 3) & BIT(7));
+ input_report_key(dev, BTN_TL, sdata_check(&sdata, 3) & BIT(0));
+ input_report_key(dev, BTN_TR, sdata_check(&sdata, 3) & BIT(1));
+ input_report_key(dev, BTN_MODE, sdata_check(&sdata, 3) & BIT(2));
if (!(xpad->mapping & MAP_STICKS_TO_NULL)) {
/* left stick */
@@ -953,11 +960,11 @@ static void xpad360_process_packet(struct usb_xpad *xpad, struct input_dev *dev,
/* triggers left/right */
if (xpad->mapping & MAP_TRIGGERS_TO_BUTTONS) {
- input_report_key(dev, BTN_TL2, data[4]);
- input_report_key(dev, BTN_TR2, data[5]);
+ input_report_key(dev, BTN_TL2, sdata_check(&sdata, 4));
+ input_report_key(dev, BTN_TR2, sdata_check(&sdata, 5));
} else {
- input_report_abs(dev, ABS_Z, data[4]);
- input_report_abs(dev, ABS_RZ, data[5]);
+ input_report_abs(dev, ABS_Z, sdata_check(&sdata, 4));
+ input_report_abs(dev, ABS_RZ, sdata_check(&sdata, 5));
}
input_sync(dev);
@@ -973,7 +980,7 @@ static void xpad360_process_packet(struct usb_xpad *xpad, struct input_dev *dev,
}
/* mode button down/up */
- if (data[3] & BIT(2))
+ if (sdata_check(&sdata, 3) & BIT(2))
xpad->mode_btn_down_ts = ktime_get_seconds();
else
xpad->mode_btn_down_ts = 0;
@@ -1019,14 +1026,18 @@ static void xpad_presence_work(struct work_struct *work)
* 01.1 - Pad state (Bytes 4+) valid
*
*/
-static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data)
+static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data, u32 len)
{
struct input_dev *dev;
+ struct safe_data sdata;
bool present;
+ sdata.data = data;
+ sdata.len = len;
+
/* Presence change */
- if (data[0] & 0x08) {
- present = (data[1] & 0x80) != 0;
+ if (sdata_check(&sdata, 0) & 0x08) {
+ present = (sdata_check(&sdata, 1) & 0x80) != 0;
if (xpad->pad_present != present) {
xpad->pad_present = present;
@@ -1035,13 +1046,13 @@ static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned cha
}
/* Valid pad data */
- if (data[1] != 0x1)
+ if (sdata_check(&sdata, 1) != 0x1)
return;
rcu_read_lock();
dev = rcu_dereference(xpad->x360w_dev);
- if (dev)
- xpad360_process_packet(xpad, dev, cmd, &data[4]);
+ if (dev && len > 4)
+ xpad360_process_packet(xpad, dev, cmd, &data[4], len - 4);
rcu_read_unlock();
}
@@ -1262,10 +1273,10 @@ static void xpad_irq_in(struct urb *urb)
switch (xpad->xtype) {
case XTYPE_XBOX360:
- xpad360_process_packet(xpad, xpad->dev, 0, xpad->idata);
+ xpad360_process_packet(xpad, xpad->dev, 0, xpad->idata, urb->actual_length);
break;
case XTYPE_XBOX360W:
- xpad360w_process_packet(xpad, 0, xpad->idata);
+ xpad360w_process_packet(xpad, 0, xpad->idata, urb->actual_length);
break;
case XTYPE_XBOXONE:
xpadone_process_packet(xpad, 0, xpad->idata, urb->actual_length);
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread