[PATCH] HID: sony: fix UAF of ghl_poke_timer / ghl_urb at driver unbind

[PATCH] HID: sony: fix UAF of ghl_poke_timer / ghl_urb at driver unbind

[Doruk Tan Ozturk]

For GHL (Guitar Hero Live) dongles, sony_probe() arms a periodic timer:
ghl_magic_poke() (the timer callback) submits sc->ghl_urb, and the URB
completion ghl_magic_poke_cb() re-arms the timer with mod_timer().

sony_remove() drained the timer with timer_delete_sync() and then freed
the URB with usb_free_urb():

	timer_delete_sync(&sc->ghl_poke_timer);
	usb_free_urb(sc->ghl_urb);

timer_delete_sync() does not block re-arming, and while the URB is in
flight the timer is not pending, so the sync delete is a no-op. A URB
completion that runs after the delete re-arms the timer, and usb_free_urb()
only drops a reference -- it does not kill an in-flight URB. sc is
allocated with devm_kzalloc() and freed once sony_remove() returns, so the
re-armed ghl_poke_timer (embedded in sc) then fires on freed memory, a
use-after-free from timer softirq. This is a disconnect/rmmod race.

Kill the URB first so no completion can re-arm the timer (further
usb_submit_urb() from the callback then fails), then timer_shutdown_sync()
to drain any last callback and permanently block re-arming, before freeing
the URB. The probe error path is unaffected: it is only reached before the
timer is armed.

Reproduced under KASAN on next-20260710 via dummy_hcd + raw-gadget
emulation of the GHL PS4 dongle (VID 0x1430 / PID 0x07bb): hid-sony binds
and arms the poke timer, the poke URB is held in flight, the driver is
unbound (freeing sc), then the URB is released. The completion re-arms the
timer on the freed sc, and the re-armed timer fires ~8 s later:

  BUG: KASAN: slab-use-after-free in ghl_magic_poke+0x98/0xb0
  Read of size 8 at addr ffff88810b02fd50 by task swapper/0/0
   ghl_magic_poke+0x98/0xb0
   call_timer_fn+0x35/0x2b0
   __run_timers+0x69c/0x9a0
   run_timer_softirq+0x173/0x2a0
  Allocated by task 169:  sony_probe
  Freed by task 338:      devres_release_group <- hid_device_remove (sony_remove)

Found by 0sec (https://0sec.ai) using automated source analysis.

Fixes: cc894ac55360 ("HID: sony: support for ghlive ps3/wii u dongles")
Cc: stable@vger.kernel.org
Assisted-by: 0sec:multi-model
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
 drivers/hid/hid-sony.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 253fff4066eb..5f9670f3910a 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -2431,7 +2431,19 @@ static void sony_remove(struct hid_device *hdev)
 	struct sony_sc *sc = hid_get_drvdata(hdev);
 
 	if (sc->quirks & (GHL_GUITAR_PS3WIIU | GHL_GUITAR_PS4)) {
-		timer_delete_sync(&sc->ghl_poke_timer);
+		/*
+		 * ghl_magic_poke() resubmits sc->ghl_urb from the poke timer
+		 * and its completion ghl_magic_poke_cb() re-arms the timer, so
+		 * an in-flight URB can re-arm the timer after a plain
+		 * timer_delete_sync() (a no-op while the URB is in flight and
+		 * the timer is not pending).  sc is devm-allocated and freed
+		 * once sony_remove() returns, so the re-armed timer would fire
+		 * on freed memory.  Kill the URB first so no completion can
+		 * re-arm the timer, then timer_shutdown_sync() to drain it and
+		 * permanently block further re-arming.
+		 */
+		usb_kill_urb(sc->ghl_urb);
+		timer_shutdown_sync(&sc->ghl_poke_timer);
 		usb_free_urb(sc->ghl_urb);
 	}
 
-- 
2.43.0

Re: [PATCH] HID: sony: fix UAF of ghl_poke_timer / ghl_urb at driver unbind

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but the patch fails to correctly fix the use-after-free race condition because it uses `usb_kill_urb()` instead of `usb_poison_urb()` or a reversed teardown sequence. `usb_kill_urb()` does not permanently prevent URB submissions; it allows the pending timer to submit the URB immediately after it returns.
--

--- Patch [1]: [PATCH] HID: sony: fix UAF of ghl_poke_timer / ghl_urb at driver unbind ---
commit 922f81b4c171a572e240262070ec65197e21cd83
Author: Doruk Tan Ozturk <doruk@0sec.ai>

HID: sony: fix UAF of ghl_poke_timer / ghl_urb at driver unbind

This commit changes the driver teardown sequence for Guitar Hero Live
dongles to fix a use-after-free during driver unbind. It attempts to kill
the in-flight URB before shutting down the timer to prevent the URB
completion from re-arming the timer on freed memory.

Fixes: cc894ac55360 ("HID: sony: support for ghlive ps3/wii u dongles")
Cc: stable@vger.kernel.org

> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index e75246d29e16d..b7b9b705b9e1c 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -2424,7 +2424,19 @@ static void sony_remove(struct hid_device *hdev)
>  	struct sony_sc *sc = hid_get_drvdata(hdev);
>  
>  	if (sc->quirks & (GHL_GUITAR_PS3WIIU | GHL_GUITAR_PS4)) {
> -		timer_delete_sync(&sc->ghl_poke_timer);
> +		/*
> +		 * ghl_magic_poke() resubmits sc->ghl_urb from the poke timer
> +		 * and its completion ghl_magic_poke_cb() re-arms the timer, so
> +		 * an in-flight URB can re-arm the timer after a plain
> +		 * timer_delete_sync() (a no-op while the URB is in flight and
> +		 * the timer is not pending).  sc is devm-allocated and freed
> +		 * once sony_remove() returns, so the re-armed timer would fire
> +		 * on freed memory.  Kill the URB first so no completion can
> +		 * re-arm the timer, then timer_shutdown_sync() to drain it and
> +		 * permanently block further re-arming.
> +		 */
> +		usb_kill_urb(sc->ghl_urb);
> +		timer_shutdown_sync(&sc->ghl_poke_timer);
> +		usb_free_urb(sc->ghl_urb);

[Severity: Critical]
This is a pre-existing issue, but does this teardown order still allow a
use-after-free race condition?

Because usb_kill_urb() does not permanently prevent URB submissions, it allows
the pending timer to submit the URB immediately after it returns.

Could this sequence occur if a user unbinds the driver while the URB is not
in flight and the ghl_poke_timer is pending:

1. usb_kill_urb() executes and returns immediately, decrementing urb->reject
   back to 0.

2. Before timer_shutdown_sync() executes, the timer expires and its callback
   ghl_magic_poke() successfully submits the URB.

3. timer_shutdown_sync() then waits for the timer callback to finish and marks
   the timer as shutdown, but the URB is now in flight.

4. The driver teardown finishes and frees sc.

5. Later, the URB completes and triggers the use-after-free by accessing the
   freed sc.

Would it be safer to use usb_poison_urb() instead to permanently block
submissions?

>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724134236.34304-1-doruk@0sec.ai?part=1