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@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.

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

thanks,

greg k-h