[PATCH] HID: sony: clean up device list on probe failure

[PATCH] HID: sony: clean up device list on probe failure

[Doruk Tan Ozturk]

sony_input_configured() adds some controllers to sony_device_list before
HID core registers their input devices. input_register_device() can fail
after the callback returns successfully. sony_probe() then observes that
HID_CLAIMED_INPUT is clear and unwinds, but only stops the HID hardware.
The devres-managed sony_sc is freed while its list node remains linked, so
the next matching controller traverses freed memory.

Initialize the list node and device ID to inactive states. Make list
removal idempotent and run the driver-private cleanup on every probe
failure path. This also makes a second cleanup safe when
sony_input_configured() already unwound a partial initialization before
sony_probe() handles the missing input claim.

Found by 0sec (https://0sec.ai) using automated source analysis;
verified against the HID input registration and probe unwind paths.

Fixes: 4f967f6d7374 ("HID: sony: Fix memory issue when connecting device using both Bluetooth and USB")
Cc: stable@vger.kernel.org
Reported-by: Doruk Tan Ozturk <doruk@0sec.ai>
Link: https://lore.kernel.org/linux-input/20260724143925.007D61F00A3A@smtp.kernel.org/
Assisted-by: 0sec:multi-model
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
 drivers/hid/hid-sony.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 253fff4066ebd..56471b7b052d1 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1982,11 +1982,10 @@ static int sony_check_add_dev_list(struct sony_sc *sc)
 
 static void sony_remove_dev_list(struct sony_sc *sc)
 {
-	if (sc->list_node.next) {
-		scoped_guard(spinlock_irqsave, &sony_dev_list_lock) {
-			list_del(&(sc->list_node));
-		}
-	}
+	guard(spinlock_irqsave)(&sony_dev_list_lock);
+
+	if (!list_empty(&sc->list_node))
+		list_del_init(&sc->list_node);
 }
 
 static int sony_get_bt_devaddr(struct sony_sc *sc)
@@ -2127,6 +2126,13 @@ static inline void sony_cancel_work_sync(struct sony_sc *sc)
 	}
 }
 
+static void sony_cleanup(struct sony_sc *sc)
+{
+	sony_cancel_work_sync(sc);
+	sony_remove_dev_list(sc);
+	sony_release_device_id(sc);
+}
+
 static int sony_input_configured(struct hid_device *hdev,
 					struct hid_input *hidinput)
 {
@@ -2313,9 +2319,7 @@ static int sony_input_configured(struct hid_device *hdev,
 err_close:
 	hid_hw_close(hdev);
 err_stop:
-	sony_cancel_work_sync(sc);
-	sony_remove_dev_list(sc);
-	sony_release_device_id(sc);
+	sony_cleanup(sc);
 	return ret;
 }
 
@@ -2339,6 +2343,8 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		return -ENOMEM;
 
 	spin_lock_init(&sc->lock);
+	INIT_LIST_HEAD(&sc->list_node);
+	sc->device_id = -1;
 
 	sc->quirks = quirks;
 	hid_set_drvdata(hdev, sc);
@@ -2367,6 +2373,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	ret = hid_hw_start(hdev, connect_mask);
 	if (ret) {
 		hid_err(hdev, "hw start failed\n");
+		sony_cleanup(sc);
 		return ret;
 	}
 
@@ -2421,7 +2428,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 err:
 	usb_free_urb(sc->ghl_urb);
-
+	sony_cleanup(sc);
 	hid_hw_stop(hdev);
 	return ret;
 }
@@ -2436,13 +2443,7 @@ static void sony_remove(struct hid_device *hdev)
 	}
 
 	hid_hw_close(hdev);
-
-	sony_cancel_work_sync(sc);
-
-	sony_remove_dev_list(sc);
-
-	sony_release_device_id(sc);
-
+	sony_cleanup(sc);
 	hid_hw_stop(hdev);
 }
 
-- 
2.43.0

Re: [PATCH] HID: sony: clean up device list on probe failure

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Missing usb_kill_urb() in sony_remove() leads to a Use-After-Free and DMA to freed memory when the URB completes.
- [Critical] Multiple HID collections in a malicious device cause permanent IDA leak and list corruption.
- [High] Unconditional calls to hid_hw_close(hdev) cause ll_open_count underflow for devices without battery support.
--

--- Patch [1]: [PATCH] HID: sony: clean up device list on probe failure ---
commit 4f91112bdbfa06050765a7259fe71fafc5ed58f1
Author: Doruk Tan Ozturk <doruk@0sec.ai>
Subject: HID: sony: clean up device list on probe failure

This commit attempts to fix a use-after-free by properly initializing the list
node and device ID, and making list removal idempotent. It introduces
sony_cleanup() to consolidate cleanup paths on probe failure and device
removal.

Link: https://lore.kernel.org/linux-input/20260724143925.007D61F00A3A@smtp.kernel.org/

> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 253fff4066ebd..56471b7b052d1 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
[ ... ]
> @@ -2127,6 +2126,13 @@ static inline void sony_cancel_work_sync(struct sony_sc *sc)
>  	}
>  }
>  
> +static void sony_cleanup(struct sony_sc *sc)
> +{
> +	sony_cancel_work_sync(sc);
> +	sony_remove_dev_list(sc);
> +	sony_release_device_id(sc);
> +}
> +
>  static int sony_input_configured(struct hid_device *hdev,
>  					struct hid_input *hidinput)
>  {

[Severity: Critical]
This is a pre-existing issue, but what happens if a malicious USB device
exposes multiple HID Application Collections?

The HID core will invoke sony_input_configured() for each collection. Since
this function isn't reentrant for the same device, won't it allocate multiple
IDAs in sony_set_device_id(), permanently leaking the previous ones?

Furthermore, wouldn't a second invocation find the device in sony_device_list
and call list_add_tail() on an already-linked node in
sony_check_add_dev_list(), corrupting the list?

[ ... ]
> @@ -2313,9 +2319,7 @@ static int sony_input_configured(struct hid_device *hdev,
>  err_close:
>  	hid_hw_close(hdev);

[Severity: High]
This isn't a bug introduced by this patch, but if sony_init_ff() fails for a
device without the SONY_BATTERY_SUPPORT quirk, won't this unconditionally call
hid_hw_close() and underflow the hdev->ll_open_count?

>  err_stop:
> -	sony_cancel_work_sync(sc);
> -	sony_remove_dev_list(sc);
> -	sony_release_device_id(sc);
> +	sony_cleanup(sc);
>  	return ret;
>  }
[ ... ]
> @@ -2436,13 +2443,7 @@ static void sony_remove(struct hid_device *hdev)
>  	}

[Severity: Critical]
This is a pre-existing issue, but shouldn't we call usb_kill_urb() before
freeing the URB in this preceding block for GHL_GUITAR devices?

If the URB is still in flight, could the completion callback
ghl_magic_poke_cb() execute asynchronously after the sony_sc structure is
freed by devres, leading to a use-after-free and timer list corruption?

>  
>  	hid_hw_close(hdev);

[Severity: High]
This is a pre-existing issue, but doesn't this unconditionally call
hid_hw_close() even if hid_hw_open() was never called (e.g., if the device
lacks SONY_BATTERY_SUPPORT)?

Would this corrupt the reference count and permanently break the hardware
state tracking if the device is later rebound via sysfs?

> -
> -	sony_cancel_work_sync(sc);
> -
> -	sony_remove_dev_list(sc);
> -
> -	sony_release_device_id(sc);
> -
> +	sony_cleanup(sc);
>  	hid_hw_stop(hdev);
>  }

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

Re: [PATCH] HID: sony: clean up device list on probe failure

[Doruk Tan Ozturk]

On Sun, Jul 26, 2026 at 01:04:00PM +0000, sashiko-bot@kernel.org wrote:
> [Critical] Multiple HID collections in a malicious device cause
> permanent IDA leak and list corruption.

Good catch. A second call to sony_input_configured() would re-allocate
the IDA and double-add to the global list. The fix is a one-line guard
checking sc->device_id >= 0, but it depends on the device_id = -1
initialization from this probe-list cleanup patch. I'll send it once
this one lands.

> [Critical] Missing usb_kill_urb() in sony_remove() leads to a
> Use-After-Free and DMA to freed memory when the URB completes.

This is the ghl_poke_timer/ghl_urb UAF I sent as a separate patch on
July 24:
https://lore.kernel.org/linux-input/20260724142703.55813-1-doruk@0sec.ai/

> [High] Unconditional calls to hid_hw_close(hdev) cause
> ll_open_count underflow for devices without battery support.

Confirmed — the err_close path calls hid_hw_close() even when the
device lacks SONY_BATTERY_SUPPORT and hid_hw_open() was never called.
A guard on the quirk would fix it. Separate issue from this patch.

-Doruk