Re: [PATCH v2] usb: misc: usbio: fix disconnect UAF in client teardown

[Hans de Goede]

Hi,

On 18-Jun-26 14:40, Cen Zhang wrote:
> usbio_disconnect() walks usbio->cli_list in reverse and uninitializes each
> auxiliary device. auxiliary_device_uninit() drops the device reference, and
> for an unbound child that can run usbio_auxdev_release() and free the
> containing struct usbio_client.
> 
> list_for_each_entry_reverse() advances after the loop body by reading
> client->link.prev. If the current client is freed by
> auxiliary_device_uninit(), the iterator dereferences freed memory.
> 
> Use list_for_each_entry_safe_reverse() so the previous client is
> cached before the body can drop the final reference. This preserves
> reverse teardown order while keeping the next iterator cursor independent
> of the current client's lifetime.
> 
> Validation reproduced this kernel report:
> BUG: KASAN: slab-use-after-free in usbio_disconnect+0x12e/0x150
> 
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0x66/0xa0
>  print_report+0xce/0x630
>  ? usbio_disconnect+0x12e/0x150
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? __virt_addr_valid+0x188/0x320
>  ? usbio_disconnect+0x12e/0x150
>  kasan_report+0xe0/0x110
>  ? usbio_disconnect+0x12e/0x150
>  usbio_disconnect+0x12e/0x150
>  usb_unbind_interface+0xf3/0x400
>  really_probe+0x316/0x660
>  __driver_probe_device+0x106/0x240
>  driver_probe_device+0x4a/0x110
>  __device_attach_driver+0xf1/0x1a0
>  ? __pfx___device_attach_driver+0x10/0x10
>  bus_for_each_drv+0xf9/0x160
>  ? __pfx_bus_for_each_drv+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? trace_hardirqs_on+0x18/0x130
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? _raw_spin_unlock_irqrestore+0x44/0x60
>  __device_attach+0x133/0x2a0
>  ? __pfx___device_attach+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? do_raw_spin_unlock+0x9a/0x100
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  device_initial_probe+0x55/0x70
>  bus_probe_device+0x4a/0xd0
>  device_add+0x9b9/0xc10
>  ? __pfx_device_add+0x10/0x10
>  ? _raw_spin_unlock_irqrestore+0x44/0x60
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? lockdep_hardirqs_on_prepare+0xea/0x1a0
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? usb_enable_lpm+0x3c/0x260
>  usb_set_configuration+0xb64/0xf20
>  usb_generic_driver_probe+0x5f/0x90
>  usb_probe_device+0x71/0x1b0
>  really_probe+0x46b/0x660
>  __driver_probe_device+0x106/0x240
>  driver_probe_device+0x4a/0x110
>  __device_attach_driver+0xf1/0x1a0
>  ? __pfx___device_attach_driver+0x10/0x10
>  bus_for_each_drv+0xf9/0x160
>  ? __pfx_bus_for_each_drv+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? trace_hardirqs_on+0x18/0x130
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? _raw_spin_unlock_irqrestore+0x44/0x60
>  __device_attach+0x133/0x2a0
>  ? __pfx___device_attach+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? do_raw_spin_unlock+0x9a/0x100
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  device_initial_probe+0x55/0x70
>  bus_probe_device+0x4a/0xd0
>  device_add+0x9b9/0xc10
>  ? __pfx_device_add+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? add_device_randomness+0xb7/0xf0
>  usb_new_device+0x492/0x870
>  hub_event+0x1b10/0x29c0
>  ? __pfx_hub_event+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? lock_acquire+0x187/0x300
>  ? process_one_work+0x475/0xb90
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? lock_release+0xc8/0x290
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  process_one_work+0x4d7/0xb90
>  ? __pfx_process_one_work+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? __list_add_valid_or_report+0x37/0xf0
>  ? __pfx_hub_event+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  worker_thread+0x2d8/0x570
>  ? __pfx_worker_thread+0x10/0x10
>  kthread+0x1ad/0x1f0
>  ? __pfx_kthread+0x10/0x10
>  ret_from_fork+0x3c9/0x540
>  ? __pfx_ret_from_fork+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? __switch_to+0x2e9/0x730
>  ? __pfx_kthread+0x10/0x10
>  ret_from_fork_asm+0x1a/0x30
>  </TASK>
> 
> Fixes: 121a0f839dbb ("usb: misc: Add Intel USBIO bridge driver")
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Cen Zhang <zzzccc427@xxxxxxxxx>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@xxxxxxxxxxxxxxxx>

Regards,

Hans



> ---
> v2:
> Drop the unnecessary list_del_init() per Hans de Goede; the safe reverse
> iterator is sufficient.
> 
>  drivers/usb/misc/usbio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/misc/usbio.c b/drivers/usb/misc/usbio.c
> index 02d1e0760f0c..0e5ccd2f04d8 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,7 @@ 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) {
>  		auxiliary_device_delete(&client->auxdev);
>  		auxiliary_device_uninit(&client->auxdev);
>  	}