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@xxxxxxxxx>
> Fixes: 4ef46367073b ("Input: xpad - fix Share button on Xbox One controllers")
> Cc: stable <stable@xxxxxxxxxx>
> Assisted-by: gkh_clanker_t1000
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
>  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