[PATCH v2] ALSA: bcd2000: Wait for open files before freeing the MIDI URBs

[PATCH v2] ALSA: bcd2000: Wait for open files before freeing the MIDI URBs

[YuanZhe Liu]

bcd2000_disconnect() calls snd_card_disconnect(), which only prevents
new opens, and then frees the MIDI URBs.  A rawmidi file that was
already open can still be closed afterwards, and its close callback,
bcd2000_midi_output_close(), calls usb_kill_urb() on midi_out_urb.

The URB pointers are cleared on disconnect and their accesses are
serialized with midi_lock, but bcd2000_midi_output_close() reads
midi_out_urb without that lock.  The check can therefore pass just
before the pointer is cleared, and usb_kill_urb() then runs on the URB
that has already been freed:

  BUG: KASAN: slab-use-after-free in usb_kill_urb+0x74/0x80
  Read of size 8 at addr ffff8880078e7d40
   usb_kill_urb
   bcd2000_midi_output_close
   close_substream
   rawmidi_release_priv
   snd_rawmidi_release

  Allocated by: usb_alloc_urb <- bcd2000_init_midi <- bcd2000_probe
  Freed by:     usb_free_urb <- bcd2000_midi_free
                <- bcd2000_disconnect

Use snd_card_disconnect_sync() instead, so that disconnect waits until
all open files are released.  The rawmidi close callbacks then run while
the URBs are still valid, and no userspace-triggered callback can run
afterwards.  The devices_mutex held by bcd2000_disconnect() is not taken
by the rawmidi close path, so the wait cannot deadlock.

Fixes: b47a22290d58 ("ALSA: MIDI driver for Behringer BCD2000 USB device")
Cc: stable@vger.kernel.org
Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn>
---
v2: rebased onto v7.3-rc7.  The URB pointer clearing and the locking are
    already handled by 459d3a64766f ("ALSA: bcd2000: clear the URB pointers
    on disconnect") and 221253723dc5 ("ALSA: bcd2000: Fix race between
    rawmidi and disconnect"), so this version only keeps the change of the
    disconnect ordering, as suggested by Takashi Iwai.

 sound/usb/bcd2000/bcd2000.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/usb/bcd2000/bcd2000.c b/sound/usb/bcd2000/bcd2000.c
index 2bd49bf82748..a5e8ea92532c 100644
--- a/sound/usb/bcd2000/bcd2000.c
+++ b/sound/usb/bcd2000/bcd2000.c
@@ -451,8 +451,13 @@ static void bcd2000_disconnect(struct usb_interface *interface)
 
 	guard(mutex)(&devices_mutex);
 
-	/* make sure that userspace cannot create new requests */
-	snd_card_disconnect(bcd2k->card);
+	/*
+	 * Make sure that userspace cannot create new requests, and wait
+	 * until all already-open files are closed so that the rawmidi
+	 * close callbacks (which may kill the out URB) run while the
+	 * URBs are still valid.
+	 */
+	snd_card_disconnect_sync(bcd2k->card);
 
 	bcd2000_free_usb_related_resources(bcd2k, interface);
 
-- 
2.34.1

[PATCH v3] ALSA: bcd2000: Fix use-after-free of MIDI URBs on file close

[YuanZhe Liu]

bcd2000_disconnect() calls snd_card_disconnect(), which only prevents
new opens, and then frees the MIDI URBs.  A rawmidi file that was
already open can still be closed afterwards, and its close callback,
bcd2000_midi_output_close(), calls usb_kill_urb() on midi_out_urb.

The URB pointers are cleared on disconnect and their accesses are
serialized with midi_lock, but bcd2000_midi_output_close() reads
midi_out_urb without that lock.  The check can therefore pass just
before the pointer is cleared, and usb_kill_urb() then runs on the URB
that has already been freed:

  BUG: KASAN: slab-use-after-free in usb_kill_urb+0x74/0x80
  Read of size 8 at addr ffff8880078e7d40
   usb_kill_urb
   bcd2000_midi_output_close
   close_substream
   rawmidi_release_priv
   snd_rawmidi_release

  Allocated by: usb_alloc_urb <- bcd2000_init_midi <- bcd2000_probe
  Freed by:     usb_free_urb <- bcd2000_midi_free
                <- bcd2000_disconnect

Take the lock in bcd2000_midi_output_close() and reference the URB with
usb_get_urb() before releasing it.  The URB then stays valid until
usb_put_urb() after usb_kill_urb() has returned, even if a disconnect
frees it in the meantime.  usb_kill_urb() itself cannot run under
midi_lock, as the completion handler takes the same lock.

Fixes: b47a22290d58 ("ALSA: MIDI driver for Behringer BCD2000 USB device")
Cc: stable@vger.kernel.org
Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn>
---
v3: reference the URB under midi_lock in bcd2000_midi_output_close()
    instead of waiting for open files in bcd2000_disconnect(), which
    would block the USB disconnect path on userspace.

 sound/usb/bcd2000/bcd2000.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/sound/usb/bcd2000/bcd2000.c b/sound/usb/bcd2000/bcd2000.c
index 2bd49bf82748..0fd086cac955 100644
--- a/sound/usb/bcd2000/bcd2000.c
+++ b/sound/usb/bcd2000/bcd2000.c
@@ -183,10 +183,24 @@ static int bcd2000_midi_output_open(struct snd_rawmidi_substream *substream)
 static int bcd2000_midi_output_close(struct snd_rawmidi_substream *substream)
 {
 	struct bcd2000 *bcd2k = substream->rmidi->private_data;
+	struct urb *urb = NULL;
 
-	if (bcd2k->midi_out_active && bcd2k->midi_out_urb) {
-		usb_kill_urb(bcd2k->midi_out_urb);
-		bcd2k->midi_out_active = 0;
+	/*
+	 * Reference the URB under the lock so that it stays valid until
+	 * usb_kill_urb() returns, even if a disconnect frees it in the
+	 * meantime.  usb_kill_urb() cannot run under the lock, as the
+	 * completion handler takes the same lock.
+	 */
+	scoped_guard(spinlock_irqsave, &bcd2k->midi_lock) {
+		if (bcd2k->midi_out_active && bcd2k->midi_out_urb) {
+			urb = usb_get_urb(bcd2k->midi_out_urb);
+			bcd2k->midi_out_active = 0;
+		}
+	}
+
+	if (urb) {
+		usb_kill_urb(urb);
+		usb_put_urb(urb);
 	}
 
 	return 0;
-- 
2.34.1

Re: [PATCH v3] ALSA: bcd2000: Fix use-after-free of MIDI URBs on file close

[Takashi Iwai]

On Sat, 31 Oct 2026 20:09:36 +0100,
YuanZhe Liu wrote:
> 
> bcd2000_disconnect() calls snd_card_disconnect(), which only prevents
> new opens, and then frees the MIDI URBs.  A rawmidi file that was
> already open can still be closed afterwards, and its close callback,
> bcd2000_midi_output_close(), calls usb_kill_urb() on midi_out_urb.
> 
> The URB pointers are cleared on disconnect and their accesses are
> serialized with midi_lock, but bcd2000_midi_output_close() reads
> midi_out_urb without that lock.  The check can therefore pass just
> before the pointer is cleared, and usb_kill_urb() then runs on the URB
> that has already been freed:
> 
>   BUG: KASAN: slab-use-after-free in usb_kill_urb+0x74/0x80
>   Read of size 8 at addr ffff8880078e7d40
>    usb_kill_urb
>    bcd2000_midi_output_close
>    close_substream
>    rawmidi_release_priv
>    snd_rawmidi_release
> 
>   Allocated by: usb_alloc_urb <- bcd2000_init_midi <- bcd2000_probe
>   Freed by:     usb_free_urb <- bcd2000_midi_free
>                 <- bcd2000_disconnect
> 
> Take the lock in bcd2000_midi_output_close() and reference the URB with
> usb_get_urb() before releasing it.  The URB then stays valid until
> usb_put_urb() after usb_kill_urb() has returned, even if a disconnect
> frees it in the meantime.  usb_kill_urb() itself cannot run under
> midi_lock, as the completion handler takes the same lock.
> 
> Fixes: b47a22290d58 ("ALSA: MIDI driver for Behringer BCD2000 USB device")
> Cc: stable@vger.kernel.org
> Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn>

Applied now.  Thanks.


Takashi