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

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

[Shengzhuo Wei]

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);
 	hid_hw_close(hdev);
 	hid_hw_stop(hdev);
 }

---
base-commit: 075b74841bd0065a3bda3440873c747938e69b68
change-id: 20260804-oxp-fix-879390c5e47f

Best regards,
-- 
Shengzhuo Wei <me@cherr.cc>

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

[Dmitry Torokhov]

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

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

[Shengzhuo Wei]

On 2026-08-03 16:33, Dmitry Torokhov wrote:
 
> What stops these jobs from re-arming? Should it use
> disable_delayed_work_sync() instead?

Agreed — cancel_delayed_work_sync() only drains the running instance; it
doesn't stop the works from being re-queued from the raw_event / sysfs /
LED-brightness paths before the device is fully torn down, so the UAF
window isn't closed. I'll switch all three calls to
disable_delayed_work_sync() in v2.

Do you have any other comments you'd like addressed in v2?

Thanks,
Shengzhuo

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

[Dmitry Torokhov]

On Tue, Aug 04, 2026 at 07:54:57AM +0800, Shengzhuo Wei wrote:
> On 2026-08-03 16:33, Dmitry Torokhov wrote:
>  
> > What stops these jobs from re-arming? Should it use
> > disable_delayed_work_sync() instead?
> 
> Agreed — cancel_delayed_work_sync() only drains the running instance; it
> doesn't stop the works from being re-queued from the raw_event / sysfs /
> LED-brightness paths before the device is fully torn down, so the UAF
> window isn't closed. I'll switch all three calls to
> disable_delayed_work_sync() in v2.
> 
> Do you have any other comments you'd like addressed in v2?

Since you are addressing work-related UAF, fixing it on probe failures
would be nice (Sashiko mentioned it).

The other Sahiko comment regarding global instance would be nice to get
fixed as well (but separate patch and only if you have
time/inclination).

Another potential change is not to create device attribute group by hand
but attach it to the driver structure so it is created and torn down
automatically, and control visibility of the attributes through
is_visible() method.

Thanks.

-- 
Dmitry