[PATCH] Input: xpad - reject short Xbox One packets before len-relative share-button index

[PATCH] Input: xpad - reject short Xbox One packets before len-relative share-button index

[Greg Kroah-Hartman]

xpadone_process_packet() receives len directly from urb->actual_length
and uses it to index the share-button byte at data[len - 18] or
data[len - 26].  Since both len and data[0] are under the device's
control, a broken controller can send a GIP_CMD_INPUT packet with
actual_length < 18 (e.g. 5 bytes) and reach this code path, causing
accesses beyond the actual array.

Since len is u32, 5 - 26 wraps to 0xFFFFFFEB, and data[0xFFFFFFEB] can
dereference about 4 GiB past the 64-byte usb_alloc_coherent() idata
buffer.  On a KASAN system this is an immediate splat otherwise the read
will either fault on an unmapped page (DoS) or pull a bit from arbitrary
kernel memory and report it as KEY_RECORD.

Fix this all up by properly bounds checking the value provided by the
device.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Fixes: 4ef46367073b ("Input: xpad - fix Share button on Xbox One controllers")
Cc: stable <stable@kernel.org>
Assisted-by: gkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/input/joystick/xpad.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index d6fc3d6006bb..7d99fe0ecf91 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1110,10 +1110,13 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
 		input_report_key(dev, BTN_START,  data[4] & BIT(2));
 		input_report_key(dev, BTN_SELECT, data[4] & BIT(3));
 		if (xpad->mapping & MAP_SHARE_BUTTON) {
-			if (xpad->mapping & MAP_SHARE_OFFSET)
-				input_report_key(dev, KEY_RECORD, data[len - 26] & BIT(0));
-			else
-				input_report_key(dev, KEY_RECORD, data[len - 18] & BIT(0));
+			if (xpad->mapping & MAP_SHARE_OFFSET) {
+				if (len >= 26)
+					input_report_key(dev, KEY_RECORD, data[len - 26] & BIT(0));
+			} else {
+				if (len >= 18)
+					input_report_key(dev, KEY_RECORD, data[len - 18] & BIT(0));
+			}
 		}
 
 		/* buttons A,B,X,Y */
-- 
2.53.0

Re: [PATCH] Input: xpad - reject short Xbox One packets before len-relative share-button index

[Dmitry Torokhov]

Hi Greg,

On Mon, Apr 20, 2026 at 05:53:15PM +0200, Greg Kroah-Hartman wrote:
> xpadone_process_packet() receives len directly from urb->actual_length
> and uses it to index the share-button byte at data[len - 18] or
> data[len - 26].  Since both len and data[0] are under the device's
> control, a broken controller can send a GIP_CMD_INPUT packet with
> actual_length < 18 (e.g. 5 bytes) and reach this code path, causing
> accesses beyond the actual array.
> 
> Since len is u32, 5 - 26 wraps to 0xFFFFFFEB, and data[0xFFFFFFEB] can
> dereference about 4 GiB past the 64-byte usb_alloc_coherent() idata
> buffer.  On a KASAN system this is an immediate splat otherwise the read
> will either fault on an unmapped page (DoS) or pull a bit from arbitrary
> kernel memory and report it as KEY_RECORD.
> 
> Fix this all up by properly bounds checking the value provided by the
> device.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Fixes: 4ef46367073b ("Input: xpad - fix Share button on Xbox One controllers")
> Cc: stable <stable@kernel.org>
> Assisted-by: gkh_clanker_t1000
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/input/joystick/xpad.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index d6fc3d6006bb..7d99fe0ecf91 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -1110,10 +1110,13 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
>  		input_report_key(dev, BTN_START,  data[4] & BIT(2));
>  		input_report_key(dev, BTN_SELECT, data[4] & BIT(3));
>  		if (xpad->mapping & MAP_SHARE_BUTTON) {
> -			if (xpad->mapping & MAP_SHARE_OFFSET)
> -				input_report_key(dev, KEY_RECORD, data[len - 26] & BIT(0));
> -			else
> -				input_report_key(dev, KEY_RECORD, data[len - 18] & BIT(0));
> +			if (xpad->mapping & MAP_SHARE_OFFSET) {
> +				if (len >= 26)
> +					input_report_key(dev, KEY_RECORD, data[len - 26] & BIT(0));
> +			} else {
> +				if (len >= 18)
> +					input_report_key(dev, KEY_RECORD, data[len - 18] & BIT(0));
> +			}

Thank you for the report, but this is quite ugly. I committed an
alternative version of the fix.

Thanks.

-- 
Dmitry

Re: [PATCH] Input: xpad - reject short Xbox One packets before len-relative share-button index

[Greg Kroah-Hartman]

On Sun, Apr 26, 2026 at 09:22:21PM -0700, Dmitry Torokhov wrote:
> Hi Greg,
> 
> On Mon, Apr 20, 2026 at 05:53:15PM +0200, Greg Kroah-Hartman wrote:
> > xpadone_process_packet() receives len directly from urb->actual_length
> > and uses it to index the share-button byte at data[len - 18] or
> > data[len - 26].  Since both len and data[0] are under the device's
> > control, a broken controller can send a GIP_CMD_INPUT packet with
> > actual_length < 18 (e.g. 5 bytes) and reach this code path, causing
> > accesses beyond the actual array.
> > 
> > Since len is u32, 5 - 26 wraps to 0xFFFFFFEB, and data[0xFFFFFFEB] can
> > dereference about 4 GiB past the 64-byte usb_alloc_coherent() idata
> > buffer.  On a KASAN system this is an immediate splat otherwise the read
> > will either fault on an unmapped page (DoS) or pull a bit from arbitrary
> > kernel memory and report it as KEY_RECORD.
> > 
> > Fix this all up by properly bounds checking the value provided by the
> > device.
> > 
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Fixes: 4ef46367073b ("Input: xpad - fix Share button on Xbox One controllers")
> > Cc: stable <stable@kernel.org>
> > Assisted-by: gkh_clanker_t1000
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  drivers/input/joystick/xpad.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> > index d6fc3d6006bb..7d99fe0ecf91 100644
> > --- a/drivers/input/joystick/xpad.c
> > +++ b/drivers/input/joystick/xpad.c
> > @@ -1110,10 +1110,13 @@ static void xpadone_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char
> >  		input_report_key(dev, BTN_START,  data[4] & BIT(2));
> >  		input_report_key(dev, BTN_SELECT, data[4] & BIT(3));
> >  		if (xpad->mapping & MAP_SHARE_BUTTON) {
> > -			if (xpad->mapping & MAP_SHARE_OFFSET)
> > -				input_report_key(dev, KEY_RECORD, data[len - 26] & BIT(0));
> > -			else
> > -				input_report_key(dev, KEY_RECORD, data[len - 18] & BIT(0));
> > +			if (xpad->mapping & MAP_SHARE_OFFSET) {
> > +				if (len >= 26)
> > +					input_report_key(dev, KEY_RECORD, data[len - 26] & BIT(0));
> > +			} else {
> > +				if (len >= 18)
> > +					input_report_key(dev, KEY_RECORD, data[len - 18] & BIT(0));
> > +			}
> 
> Thank you for the report, but this is quite ugly. I committed an
> alternative version of the fix.

Not a problem at all, thanks for doing this, all I want is to see the
issue fixed :)

thanks,

greg k-h