[PATCH 1/5] ALSA: usx2y: us144mkii: fix stop_pcm_work race with PCM close

[Alexandre Becholey <alex@becholey.ch>]

The feedback URB completion handler schedules stop_pcm_work when the
device stops sending valid feedback. The work handler then calls
snd_pcm_stop() on tascam->playback_substream and
tascam->capture_substream without the stream lock, and nothing keeps
these substreams open while it runs: the PCM .close callbacks only clear
the pointers and do not wait for the work.

If the PCM is closed while the work is running, snd_pcm_detach_substream()
frees the runtime under it, and the work then accesses the freed runtime
in snd_pcm_post_stop(), or dereferences a NULL substream->runtime in
snd_pcm_do_stop(), depending on how far the close has got. A USB device
can fire the work whenever it wants by sending bad feedback packets, so
a malicious device plus a process closing the PCM can trigger this.
With KASAN, on torvalds/master 067c5c24cef2:

  BUG: KASAN: slab-use-after-free in _raw_spin_lock_irqsave+0xb0/0x104
  Workqueue: events tascam_stop_pcm_work_handler

Clear the substream pointers under tascam->lock in the .close callbacks
and flush stop_pcm_work afterwards, so the work can no longer run on a
closing substream. In the work, read the pointers under the same lock
and stop the streams with snd_pcm_stop_xrun(), which takes the stream
lock and checks that the stream is still running. The lock is dropped
before snd_pcm_stop_xrun() because the trigger callback takes
tascam->lock with the stream lock held.

Found with an emulated US-144MKII device (USB/IP, vhci_hcd).

The bug was found, and the reproducer and this fix were written, by AI
agents (Claude Mythos and Claude Opus models). The analysis and the
patch were reviewed by a human.

Reported-by: Hanxiang Xu <xuhx@hust.edu.cn>
Closes: https://lore.kernel.org/all/6891aa7d.26365.1a1d4e2c4e4.Coremail.xuhx@hust.edu.cn/
Fixes: a2a2210f2c2e ("ALSA: usb-audio: us144mkii: Implement audio playback and feedback")
Cc: stable@vger.kernel.org
Signed-off-by: Alexandre Becholey <alex@becholey.ch>
---
Notes for reviewers (not part of the commit message):

Trigger: an emulated TASCAM US-144MKII served over USB/IP to vhci_hcd
(an AF_UNIX socketpair inside the VM; dummy_hcd cannot do isochronous
transfers) under QEMU, arm64, KASAN. The program opens a PCM and
starts streaming; the device sends invalid feedback packets until the
driver logs "Fatal: Feedback sync lost. Stopping stream." and schedules
stop_pcm_work, while the program closes the PCM. The reproducer is available to maintainers on request.

Decoded trace, torvalds/master 067c5c24cef2 (arm64, QEMU virt); the
object is the kmalloc-2k snd_pcm_runtime, allocated by
snd_pcm_attach_substream() on open and freed by
snd_pcm_detach_substream() from snd_pcm_release() on close:

  BUG: KASAN: slab-use-after-free in _raw_spin_lock_irqsave (include/linux/spinlock_api_smp.h:155 (discriminator 1) kernel/locking/spinlock.c:181 (discriminator 1))
  Read of size 1 at addr ffff00001318a180 by task kworker/1:0/24
  CPU: 1 UID: 0 PID: 24 Comm: kworker/1:0 Not tainted 7.4.0-rc2+ #1 PREEMPT(full)
  Hardware name: linux,dummy-virt (DT)
  Workqueue: events tascam_stop_pcm_work_handler
  Call trace:
   show_stack (arch/arm64/kernel/stacktrace.c:498 arch/arm64/kernel/stacktrace.c:499) (C)
   dump_stack_lvl (lib/dump_stack.c:94 lib/dump_stack.c:120)
   print_report (mm/kasan/report.c:378 mm/kasan/report.c:482)
   kasan_report (mm/kasan/report.c:595)
   __kasan_check_byte (mm/kasan/common.c:585)
   lock_acquire (include/linux/kasan.h:402 (discriminator 1) kernel/locking/lockdep.c:5916 (discriminator 1) kernel/locking/lockdep.c:5899 (discriminator 1))
   _raw_spin_lock_irqsave (include/linux/spinlock_api_smp.h:155 (discriminator 1) kernel/locking/spinlock.c:181 (discriminator 1))
   __wake_up (kernel/sched/wait.c:124 (discriminator 3) kernel/sched/wait.c:146 (discriminator 3))
   snd_pcm_post_stop (sound/core/pcm_native.c:1622)
   snd_pcm_action_single (sound/core/pcm_native.c:1386)
   snd_pcm_action (sound/core/pcm_native.c:1467)
   snd_pcm_stop (sound/core/pcm_native.c:1642)
   tascam_stop_pcm_work_handler (sound/usb/usx2y/us144mkii_playback.c:453)
   process_one_work (kernel/workqueue.c:3495)
   worker_thread (kernel/workqueue.c:3578 kernel/workqueue.c:3659)
   kthread (kernel/kthread.c:436)
   ret_from_fork (arch/arm64/kernel/entry.S:870)

On v7.3.1 the same reproducer also ends in
"KASAN: null-ptr-deref in range [0x1c0-0x1c7]" in snd_pcm_do_stop()
called from tascam_stop_pcm_work_handler() (us144mkii_playback.c:453),
the snd_pcm_stop() call this patch replaces.

sound/usb/usx2y/us144mkii*.c are identical in v7.3.1, 067c5c24cef2
and 9d66f26fefac.

Testing, KASAN: without the patch every run crashed in
tascam_stop_pcm_work_handler (v7.3.1 3/3, 067c5c24cef2 1/1). With the
patch no run showed this crash (v7.3.1 0/14 with three test programs,
067c5c24cef2 0/5 with one).
With patches 1/5..3/5 applied (v7.3.1, the same three test programs,
8 runs), the stop work fired 10 to 1740 times per run and no run
showed this crash; 4 of those 8 runs ended in the first two other
problems listed below. These programs were not re-run with 4/5 and
5/5 added; those patches were tested with their own reproducers.

17 of the 19 runs with this patch alone still crashed elsewhere:
- 12 runs: list_del corruption in __usb_unanchor_urb(), on v7.3.1
  after a WARNING at drivers/usb/core/urb.c:379 in the stream start
  path (that WARNING was also seen without the patch); fixed by
  patch 5/5;
- 4 runs: KASAN use-after-free in vhci_urb_dequeue() from
  usb_kill_anchored_urbs() in tascam_stop_work_handler(), a race in
  the vhci_hcd test harness, not in this driver;
- 1 run: KASAN slab-use-after-free in feedback_urb_complete()
  (us144mkii_playback.c:347, reading playback_rt->rate); fixed by
  patch 4/5.

Thanks,
Alexandre Becholey

 sound/usb/usx2y/us144mkii_capture.c  |  5 ++++-
 sound/usb/usx2y/us144mkii_playback.c | 19 ++++++++++++++-----
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/sound/usb/usx2y/us144mkii_capture.c b/sound/usb/usx2y/us144mkii_capture.c
index d7cdf9e0f685..30cdd73ca99c 100644
--- a/sound/usb/usx2y/us144mkii_capture.c
+++ b/sound/usb/usx2y/us144mkii_capture.c
@@ -36,7 +36,10 @@ static int tascam_capture_close(struct snd_pcm_substream *substream)
 {
 	struct tascam_card *tascam = snd_pcm_substream_chip(substream);
 
-	tascam->capture_substream = NULL;
+	scoped_guard(spinlock_irqsave, &tascam->lock)
+		tascam->capture_substream = NULL;
+	/* The stop work may still be using this substream; wait for it. */
+	flush_work(&tascam->stop_pcm_work);
 
 	return 0;
 }
diff --git a/sound/usb/usx2y/us144mkii_playback.c b/sound/usb/usx2y/us144mkii_playback.c
index 7efaca0a6489..0df95dbf585d 100644
--- a/sound/usb/usx2y/us144mkii_playback.c
+++ b/sound/usb/usx2y/us144mkii_playback.c
@@ -36,7 +36,10 @@ static int tascam_playback_close(struct snd_pcm_substream *substream)
 {
 	struct tascam_card *tascam = snd_pcm_substream_chip(substream);
 
-	tascam->playback_substream = NULL;
+	scoped_guard(spinlock_irqsave, &tascam->lock)
+		tascam->playback_substream = NULL;
+	/* The stop work may still be using this substream; wait for it. */
+	flush_work(&tascam->stop_pcm_work);
 
 	return 0;
 }
@@ -448,9 +451,15 @@ void tascam_stop_pcm_work_handler(struct work_struct *work)
 {
 	struct tascam_card *tascam =
 		container_of(work, struct tascam_card, stop_pcm_work);
+	struct snd_pcm_substream *playback_ss, *capture_ss;
+
+	scoped_guard(spinlock_irqsave, &tascam->lock) {
+		playback_ss = tascam->playback_substream;
+		capture_ss = tascam->capture_substream;
+	}
 
-	if (tascam->playback_substream)
-		snd_pcm_stop(tascam->playback_substream, SNDRV_PCM_STATE_XRUN);
-	if (tascam->capture_substream)
-		snd_pcm_stop(tascam->capture_substream, SNDRV_PCM_STATE_XRUN);
+	if (playback_ss)
+		snd_pcm_stop_xrun(playback_ss);
+	if (capture_ss)
+		snd_pcm_stop_xrun(capture_ss);
 }