Re: [PATCH] ALSA: usb-audio: Kill MIDI 2.0 URBs before freeing endpoints

From: Takashi Iwai

Date: Thu Jun 18 2026 - 06:36:05 EST


On Thu, 18 Jun 2026 12:02:27 +0200,
Cen Zhang wrote:
>
> MIDI 2.0 input URBs are started during snd_usb_midi_v2_create().
> A later setup failure can still jump to snd_usb_midi_v2_free(), which
> currently frees each endpoint and its coherent URB buffers without first
> stopping the submitted URBs. A completion can then dereference the
> embedded URB context and endpoint state after they have been freed, or
> try to resubmit from the stale endpoint.
>
> The buggy scenario involves two paths, with each column showing the
> order within that path:
>
> probe error path: USB completion path:
> 1. start_input_streams() submits 1. The HCD still owns a
> input URBs. submitted input URB.
> 2. A later setup helper returns 2. input_urb_complete() runs
> an error. with urb->context in ep.
> 3. snd_usb_midi_v2_free() frees 3. The completion reads ep
> endpoint storage and URB buffers. state and can requeue URBs.
>
> Make the endpoint destructor follow the same teardown ordering used for
> disconnect: publish ep->disconnected, kill the URBs synchronously, and
> drain the endpoint before freeing URB buffers and endpoint storage.
>
> Validation reproduced this kernel report:
> BUG: KASAN: slab-use-after-free in input_urb_complete+0x37/0x1b0
> Workqueue: usb_hub_wq hub_event
> RIP: 0010:_raw_spin_unlock_irq+0x2e/0x50
> Read of size 8
> Call trace:
> dump_stack_lvl+0x77/0xb0
> print_report+0xce/0x5f0
> input_urb_complete+0x37/0x1b0 (sound/usb/midi2.c:186)
> srso_alias_return_thunk+0x5/0xfbef5
> __virt_addr_valid+0x19f/0x330
> kasan_report+0xe0/0x110
> __usb_hcd_giveback_urb+0x112/0x1d0
> dummy_timer+0xaaa/0x19a0
> lock_is_held_type+0x9a/0x110
> __lock_acquire+0x467/0x28b0
> mark_held_locks+0x40/0x70
> _raw_spin_unlock_irqrestore+0x44/0x60
> lockdep_hardirqs_on_prepare+0xbb/0x1a0
> __hrtimer_run_queues+0x101/0x520
> hrtimer_run_softirq+0xd0/0x130
> handle_softirqs+0x15b/0x670
> __irq_exit_rcu+0xd0/0x170
> irq_exit_rcu+0xe/0x20
> sysvec_apic_timer_interrupt+0x6c/0x80
> asm_sysvec_apic_timer_interrupt+0x1a/0x20
>
> Fixes: d9c99876868c ("ALSA: usb-audio: Create UMP blocks from USB MIDI GTBs")
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Cen Zhang <zzzccc427@xxxxxxxxx>
> ---
> sound/usb/midi2.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/sound/usb/midi2.c b/sound/usb/midi2.c
> index 04aeb9052f13..94c1a52853e0 100644
> --- a/sound/usb/midi2.c
> +++ b/sound/usb/midi2.c
> @@ -470,6 +470,9 @@ static int create_midi2_endpoint(struct snd_usb_midi2_interface *umidi,
> static void free_midi2_endpoint(struct snd_usb_midi2_endpoint *ep)
> {
> list_del(&ep->list);
> + ep->disconnected = 1;
> + kill_midi_urbs(ep, false);
> + drain_urb_queue(ep);
> free_midi_urbs(ep);
> kfree(ep);
> }

Thanks for the patch. This can be good as a quick paper-over, so that
it can be easily backported. But the proper fix would be rather to
call the core part of __usb_audio_disconnect() at the error path
(something like below -- based on the latest tree).

That said, I can take your patch as a quick fix, but could you correct
the patch for avoiding the doubly disconnection procedure? The code
should have a check of ep->disconnected, such as:

static void free_midi2_endpoint(struct snd_usb_midi2_endpoint *ep)
{
list_del(&ep->list);
if (!ep->disconnected) {
ep->disconnected = 1;
kill_midi_urbs(ep, false);
drain_urb_queue(ep);
}
free_midi_urbs(ep);
kfree(ep);
}


Takashi

-- 8< --
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -905,6 +905,41 @@ static int try_to_register_card(struct snd_usb_audio *chip, int ifnum)
return 0;
}

+/* disconnect / release various resources assigned to the chip */
+static void usb_audio_disconnect_components(struct snd_usb_audio *chip)
+{
+ struct list_head *p;
+ struct snd_usb_stream *as;
+ struct snd_usb_endpoint *ep;
+ struct usb_mixer_interface *mixer;
+
+ /* release the pcm resources */
+ list_for_each_entry(as, &chip->pcm_list, list)
+ snd_usb_stream_disconnect(as);
+
+ /* release the endpoint resources */
+ list_for_each_entry(ep, &chip->ep_list, list)
+ snd_usb_endpoint_release(ep);
+
+ /* release the midi resources */
+ list_for_each(p, &chip->midi_list)
+ snd_usbmidi_disconnect(p);
+
+ snd_usb_midi_v2_disconnect_all(chip);
+
+ /*
+ * Nice to check quirk && quirk->shares_media_device and
+ * then call the snd_media_device_delete(). Don't have
+ * access to the quirk here. snd_media_device_delete()
+ * accesses mixer_list
+ */
+ snd_media_device_delete(chip);
+
+ /* release mixer resources */
+ list_for_each_entry(mixer, &chip->mixer_list, list)
+ snd_usb_mixer_disconnect(mixer);
+}
+
/*
* probe the active usb device
*
@@ -1077,8 +1112,10 @@ static int usb_audio_probe(struct usb_interface *intf,
* decrement before memory is possibly returned.
*/
atomic_dec(&chip->active);
- if (!chip->num_interfaces)
+ if (!chip->num_interfaces) {
+ usb_audio_disconnect_components(chip);
snd_card_free(chip->card);
+ }
}
return err;
}
@@ -1091,48 +1128,19 @@ static bool __usb_audio_disconnect(struct usb_interface *intf,
struct snd_usb_audio *chip,
struct snd_card *card)
{
- struct list_head *p;
-
guard(mutex)(&register_mutex);

if (platform_ops && platform_ops->disconnect_cb)
platform_ops->disconnect_cb(chip);

if (atomic_inc_return(&chip->shutdown) == 1) {
- struct snd_usb_stream *as;
- struct snd_usb_endpoint *ep;
- struct usb_mixer_interface *mixer;
-
/* wait until all pending tasks done;
* they are protected by snd_usb_lock_shutdown()
*/
snd_refcount_sync(&chip->usage_count);
snd_card_disconnect(card);
- /* release the pcm resources */
- list_for_each_entry(as, &chip->pcm_list, list) {
- snd_usb_stream_disconnect(as);
- }
- /* release the endpoint resources */
- list_for_each_entry(ep, &chip->ep_list, list) {
- snd_usb_endpoint_release(ep);
- }
- /* release the midi resources */
- list_for_each(p, &chip->midi_list) {
- snd_usbmidi_disconnect(p);
- }
- snd_usb_midi_v2_disconnect_all(chip);
- /*
- * Nice to check quirk && quirk->shares_media_device and
- * then call the snd_media_device_delete(). Don't have
- * access to the quirk here. snd_media_device_delete()
- * accesses mixer_list
- */
- snd_media_device_delete(chip);

- /* release mixer resources */
- list_for_each_entry(mixer, &chip->mixer_list, list) {
- snd_usb_mixer_disconnect(mixer);
- }
+ usb_audio_disconnect_components(chip);
}

if (chip->quirk_flags & QUIRK_FLAG_DISABLE_AUTOSUSPEND)