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

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

[Cen Zhang]

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@gmail.com>
---
 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);
 }
-- 
2.43.0

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

[Takashi Iwai]

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@gmail.com>
> ---
>  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)

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

[Cen Zhang]

Hi, Takashi

> 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:
>

Thanks for your review and guidence. I followed your suggestion and
reworked the fix accordingly:
the common component cleanup is now shared with the probe error path, and the
MIDI 2.0 endpoint destructor keeps the ep->disconnected guard to avoid double
disconnect.

Best regards,
Cen Zhang