Re: [PATCH] HID: hid-oxp: fix UAF on pending work in remove()

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Shengzhuo,

On Tue, Aug 04, 2026 at 07:15:32AM +0800, Shengzhuo Wei wrote:
> oxp_cfg_probe() arms drvdata.oxp_mcu_init to run 50 ms after probe, and
> oxp_mcu_init_fn() dereferences drvdata.hdev to issue MCU output reports
> (oxp_set_buttons()/oxp_gen_2_property_out() -> hid_hw_output_report(),
> each followed by msleep(200)). oxp_hid_remove() cancels it with the
> non-synchronising cancel_delayed_work(), so a worker already running is
> not waited for; removing the device while the worker is asleep then frees
> the hid_device underneath it, leaving drvdata.hdev stale -- a
> use-after-free when the worker wakes.
> 
> The oxp_rgb_queue and oxp_btn_queue workers, wired up the same way and
> also cancelled with cancel_delayed_work() in oxp_hid_remove(), have the
> same problem.
> 
> Drain all three works with cancel_delayed_work_sync() in oxp_hid_remove()
> so they have exited before the hid_device is freed.
> 
> Fixes: 84910c459d65 ("HID: hid-oxp: Add OneXPlayer configuration driver")
> Fixes: e4c850a6e750 ("HID: hid-oxp: Add Button Mapping Interface")
> Fixes: 2f424f28fb39 ("HID: hid-oxp: Add Second Generation Gamepad Mode Switch")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shengzhuo Wei <me@cherr.cc>
> ---
> Same delayed-work use-after-free class as the 7.2-rc6 sweep
> (hid-lenovo-go, hid-lenovo-go-s, hid-lg-g15, hid-appleir, hid-letsketch);
> hid-oxp was missed. The fix mirrors the cancel_delayed_work_sync()
> approach already used by hid-lenovo-go / hid-lenovo-go-s.
> ---
>  drivers/hid/hid-oxp.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c
> index 20a54f337220dc2aee3483a14d542b66c487bd60..d8fb6a69d40d43f2595179df1067d42b4b3e166a 100644
> --- a/drivers/hid/hid-oxp.c
> +++ b/drivers/hid/hid-oxp.c
> @@ -1552,9 +1552,9 @@ static int oxp_hid_probe(struct hid_device *hdev,
>  
>  static void oxp_hid_remove(struct hid_device *hdev)
>  {
> -	cancel_delayed_work(&drvdata.oxp_rgb_queue);
> -	cancel_delayed_work(&drvdata.oxp_btn_queue);
> -	cancel_delayed_work(&drvdata.oxp_mcu_init);
> +	cancel_delayed_work_sync(&drvdata.oxp_rgb_queue);
> +	cancel_delayed_work_sync(&drvdata.oxp_btn_queue);
> +	cancel_delayed_work_sync(&drvdata.oxp_mcu_init);

What stops these jobs from re-arming? Should it use
disable_delayed_work_sync() instead?

Thanks.

-- 
Dmitry