Re: [PATCH 2/2] Input: xpad - add wired Beitong BTP-KP20D support

On Sat, Jul 25, 2026 at 3:48=E2=80=AFPM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issu=
e(s) to consider:
>
> New issues:
> - [High] Probe error path after input_register_device() causes a Use-Afte=
r-Free risk because xpad driver data is unconditionally freed while the inp=
ut device is visible to userspace.
>
> Pre-existing issues:
> - [Critical] The probe error path frees the output URB (irq_out) and its =
DMA buffer without killing the active URB, causing memory corruption.
> --
>
> --- Patch [2]: [PATCH 2/2] Input: xpad - add wired Beitong BTP-KP20D supp=
ort ---
> commit ec428f1a8b8e609c623920e0839f0af31cf70bb0
> Author: Aaron Ma <[email protected]>
>
> Input: xpad - add wired Beitong BTP-KP20D support
>
> The wired BTP-KP20D resets itself when its interrupt-IN pipe is idle
> for a few seconds, causing periodic re-enumeration whenever no
> userspace client holds the input device open. Add a FLAG_KEEPALIVE
> quirk that keeps the input URB running from probe to disconnect,
> mirroring the XTYPE_XBOX360W URB lifecycle.
>
> > diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpa=
d.c
> > index 1616047e65f5e..5369b880c6a08 100644
> > --- a/drivers/input/joystick/xpad.c
> > +++ b/drivers/input/joystick/xpad.c
>
> [ ... ]
>
> > @@ -2227,9 +2232,22 @@ static int xpad_probe(struct usb_interface *intf=
, const struct usb_device_id *id
> >               error =3D xpad_init_input(xpad);
> >               if (error)
> >                       goto err_deinit_output;
> > +
> > +             if (xpad->keepalive) {
> > +                     /*
> > +                      * The device resets itself when the interrupt-IN=
 pipe
> > +                      * is idle for a few seconds, so keep the URB run=
ning
> > +                      * from probe until disconnect.
> > +                      */
> > +                     error =3D xpad_start_input(xpad);
> > +                     if (error)
> > +                             goto err_deinit_input;
>
> [Severity: High]
> If xpad_start_input() fails here, we jump to err_deinit_input which
> unregisters the input device, and eventually falls through to kfree(xpad)
> in xpad_probe().
>
> Because xpad_init_input() has already successfully called
> input_register_device(), the device is visible to userspace.
>
> Could a local user open the evdev node immediately after registration and
> issue an EVIOCSFF ioctl concurrently? If so, input_ff_upload() would call
> xpad_play_effect(), which might dereference xpad after it has been freed
> since input_unregister_device() does not block concurrent ioctls that
> have already passed the evdev->exist check.
>

The EVIOCSFF UAF is not reachable. EVIOCSFF calls input_ff_upload(),
whose memless upload callback is ml_ff_upload(), not xpad_play_effect().
xpad_play_effect() is called later from the memless FF timer. During
unregistration, ml_ff_stop() calls timer_shutdown_sync() before xpad is
freed, so that callback cannot outlive xpad.



> > +             }
> >       }
> >       return 0;
> >
> > +err_deinit_input:
> > +     xpad_deinit_input(xpad);
> >  err_deinit_output:
> >       xpad_deinit_output(xpad);
>
> [Severity: Critical]
> This is a pre-existing issue, but this new fallible xpad_start_input()
> call creates a new way to trigger it.
>
> If we take the error path to err_deinit_output after xpad_init_input()
> succeeds (which calls xpad_led_probe() and submits the irq_out URB for
> Xbox 360 controllers), xpad_deinit_output() will free the irq_out URB
> and its odata DMA buffer.
>
> However, it does not appear to call xpad_stop_output() or usb_kill_urb().
> Can this lead to memory corruption if the USB host controller continues
> writing to the freed odata DMA buffer, and the URB completion handler
> xpad_irq_out() executes and dereferences the freed xpad structure?
>

The output URB issue is valid. I added xpad_stop_output() before
xpad_deinit_output() in the probe error path, matching xpad_disconnect().

Thanks,
Aaron

> >  err_free_in_urb:
> >       usb_free_urb(xpad->irq_in);
>
> --
> Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260725072813.33=
[email protected]?part=3D2