Linux USB
 help / color / mirror / Atom feed
* [PATCH] usb: misc: usbio: fix disconnect UAF in client teardown
@ 2026-06-18 10:46 Cen Zhang
  2026-06-18 11:22 ` Hans de Goede
  0 siblings, 1 reply; 3+ messages in thread
From: Cen Zhang @ 2026-06-18 10:46 UTC (permalink / raw)
  To: Israel Cepeda, Hans de Goede, Sakari Ailus, Greg Kroah-Hartman
  Cc: linux-usb, linux-kernel, baijiaju1990, zzzccc427

usbio_disconnect() walks usbio->cli_list in reverse while each
auxiliary_device_uninit() can drop the last device reference and run
usbio_auxdev_release(). If that happens, the current struct usbio_client is
freed before list_for_each_entry_reverse() advances by reading
client->link.prev.

Use list_for_each_entry_safe_reverse() and delete the list node before
uninitializing the auxiliary device. The next cursor is then captured before
the put_device() path can free the current client, and cli_list does not retain
stale nodes during teardown.

Validation reproduced this kernel report:
KASAN slab-use-after-free in usbio_disconnect+0x12e/0x150
Workqueue: usb_hub_wq hub_event
Read of size 8
Call trace:
  dump_stack_lvl+0x66/0xa0
  print_report+0xce/0x630
  usbio_disconnect+0x12e/0x150 (drivers/usb/misc/usbio.c:518)
  srso_alias_return_thunk+0x5/0xfbef5
  __virt_addr_valid+0x188/0x320
  kasan_report+0xe0/0x110
  usb_unbind_interface+0xf3/0x400
  __device_attach_driver+0xf1/0x1a0
  bus_for_each_drv+0xf9/0x160
  trace_hardirqs_on+0x18/0x130
  _raw_spin_unlock_irqrestore+0x44/0x60
  __device_attach+0x133/0x2a0
  do_raw_spin_unlock+0x9a/0x100
  device_add+0x9b9/0xc10
  lockdep_hardirqs_on_prepare+0xea/0x1a0
  usb_enable_lpm+0x3c/0x260
  usb_set_configuration+0xb64/0xf20
  add_device_randomness+0xb7/0xf0
  usb_new_device+0x492/0x870
  hub_event+0x1b10/0x29c0
  lock_acquire+0x187/0x300
  process_one_work+0x475/0xb90 (kernel/workqueue.c:3200)
  lock_release+0xc8/0x290
  process_one_work+0x4d7/0xb90 (kernel/workqueue.c:3200)
  __list_add_valid_or_report+0x37/0xf0
  worker_thread+0x2d8/0x570
  kthread+0x1ad/0x1f0
  ret_from_fork+0x3c9/0x540
  __switch_to+0x2e9/0x730
  ret_from_fork_asm+0x1a/0x30

Fixes: 121a0f839dbb ("usb: misc: Add Intel USBIO bridge driver")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 drivers/usb/misc/usbio.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/misc/usbio.c b/drivers/usb/misc/usbio.c
index 02d1e0760f0c..7dc44bbcafd2 100644
--- a/drivers/usb/misc/usbio.c
+++ b/drivers/usb/misc/usbio.c
@@ -518,7 +518,7 @@ static int usbio_resume(struct usb_interface *intf)
 static void usbio_disconnect(struct usb_interface *intf)
 {
 	struct usbio_device *usbio = usb_get_intfdata(intf);
-	struct usbio_client *client;
+	struct usbio_client *client, *next;
 
 	/* Wakeup any clients waiting for a reply */
 	usbio->rxdat_len = 0;
@@ -535,7 +535,8 @@ static void usbio_disconnect(struct usb_interface *intf)
 	usb_kill_urb(usbio->urb);
 	usb_free_urb(usbio->urb);
 
-	list_for_each_entry_reverse(client, &usbio->cli_list, link) {
+	list_for_each_entry_safe_reverse(client, next, &usbio->cli_list, link) {
+		list_del_init(&client->link);
 		auxiliary_device_delete(&client->auxdev);
 		auxiliary_device_uninit(&client->auxdev);
 	}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] usb: misc: usbio: fix disconnect UAF in client teardown
  2026-06-18 10:46 [PATCH] usb: misc: usbio: fix disconnect UAF in client teardown Cen Zhang
@ 2026-06-18 11:22 ` Hans de Goede
  2026-06-18 12:24   ` Cen Zhang
  0 siblings, 1 reply; 3+ messages in thread
From: Hans de Goede @ 2026-06-18 11:22 UTC (permalink / raw)
  To: Cen Zhang, Israel Cepeda, Sakari Ailus, Greg Kroah-Hartman
  Cc: linux-usb, linux-kernel, baijiaju1990

Hi,

On 18-Jun-26 12:46, Cen Zhang wrote:
> usbio_disconnect() walks usbio->cli_list in reverse while each
> auxiliary_device_uninit() can drop the last device reference and run
> usbio_auxdev_release(). If that happens, the current struct usbio_client is
> freed before list_for_each_entry_reverse() advances by reading
> client->link.prev.
> 
> Use list_for_each_entry_safe_reverse()

Ack that is good.

> and delete the list node before
> uninitializing the auxiliary device.

Why I see no reason to delete the list node here ?

Regards,

Hans




> The next cursor is then captured before
> the put_device() path can free the current client, and cli_list does not retain
> stale nodes during teardown.
> 
> Validation reproduced this kernel report:
> KASAN slab-use-after-free in usbio_disconnect+0x12e/0x150
> Workqueue: usb_hub_wq hub_event
> Read of size 8
> Call trace:
>   dump_stack_lvl+0x66/0xa0
>   print_report+0xce/0x630
>   usbio_disconnect+0x12e/0x150 (drivers/usb/misc/usbio.c:518)
>   srso_alias_return_thunk+0x5/0xfbef5
>   __virt_addr_valid+0x188/0x320
>   kasan_report+0xe0/0x110
>   usb_unbind_interface+0xf3/0x400
>   __device_attach_driver+0xf1/0x1a0
>   bus_for_each_drv+0xf9/0x160
>   trace_hardirqs_on+0x18/0x130
>   _raw_spin_unlock_irqrestore+0x44/0x60
>   __device_attach+0x133/0x2a0
>   do_raw_spin_unlock+0x9a/0x100
>   device_add+0x9b9/0xc10
>   lockdep_hardirqs_on_prepare+0xea/0x1a0
>   usb_enable_lpm+0x3c/0x260
>   usb_set_configuration+0xb64/0xf20
>   add_device_randomness+0xb7/0xf0
>   usb_new_device+0x492/0x870
>   hub_event+0x1b10/0x29c0
>   lock_acquire+0x187/0x300
>   process_one_work+0x475/0xb90 (kernel/workqueue.c:3200)
>   lock_release+0xc8/0x290
>   process_one_work+0x4d7/0xb90 (kernel/workqueue.c:3200)
>   __list_add_valid_or_report+0x37/0xf0
>   worker_thread+0x2d8/0x570
>   kthread+0x1ad/0x1f0
>   ret_from_fork+0x3c9/0x540
>   __switch_to+0x2e9/0x730
>   ret_from_fork_asm+0x1a/0x30
> 
> Fixes: 121a0f839dbb ("usb: misc: Add Intel USBIO bridge driver")
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
>  drivers/usb/misc/usbio.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/misc/usbio.c b/drivers/usb/misc/usbio.c
> index 02d1e0760f0c..7dc44bbcafd2 100644
> --- a/drivers/usb/misc/usbio.c
> +++ b/drivers/usb/misc/usbio.c
> @@ -518,7 +518,7 @@ static int usbio_resume(struct usb_interface *intf)
>  static void usbio_disconnect(struct usb_interface *intf)
>  {
>  	struct usbio_device *usbio = usb_get_intfdata(intf);
> -	struct usbio_client *client;
> +	struct usbio_client *client, *next;
>  
>  	/* Wakeup any clients waiting for a reply */
>  	usbio->rxdat_len = 0;
> @@ -535,7 +535,8 @@ static void usbio_disconnect(struct usb_interface *intf)
>  	usb_kill_urb(usbio->urb);
>  	usb_free_urb(usbio->urb);
>  
> -	list_for_each_entry_reverse(client, &usbio->cli_list, link) {
> +	list_for_each_entry_safe_reverse(client, next, &usbio->cli_list, link) {
> +		list_del_init(&client->link);
>  		auxiliary_device_delete(&client->auxdev);
>  		auxiliary_device_uninit(&client->auxdev);
>  	}


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] usb: misc: usbio: fix disconnect UAF in client teardown
  2026-06-18 11:22 ` Hans de Goede
@ 2026-06-18 12:24   ` Cen Zhang
  0 siblings, 0 replies; 3+ messages in thread
From: Cen Zhang @ 2026-06-18 12:24 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Israel Cepeda, Sakari Ailus, Greg Kroah-Hartman, linux-usb,
	linux-kernel, baijiaju1990

Hi, Hans

> Why I see no reason to delete the list node here ?

Thanks a lot for your review. You are right, the list_del_init() is
not needed for fixing this.

I'll send a v2 dropping the list_del_init() and update the commit message
accordingly.

Best regards,
Cen Zhang

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-06-18 12:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-18 10:46 [PATCH] usb: misc: usbio: fix disconnect UAF in client teardown Cen Zhang
2026-06-18 11:22 ` Hans de Goede
2026-06-18 12:24   ` Cen Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox