ALSA: usb-audio: us144mkii: queued stop_pcm_work calls snd_pcm_stop() after PCM close and dereferences NULL runtime

[徐晗翔 <xuhx@hust.edu.cn>]

[-- Attachment #1: Type: text/plain, Size: 8406 bytes --]

tascam_stop_pcm_work_handler() is queued by feedback_urb_complete()
after a feedback sync loss, then reads tascam->playback_substream and
tascam->capture_substream without any lock and calls snd_pcm_stop() on
them.  PCM close does not cancel this work.  If close completes first,
snd_pcm_detach_substream() sets substream->runtime to NULL and frees the
snd_pcm_runtime; when the handler reaches snd_pcm_stop(),
snd_pcm_pre_stop() re-reads substream->runtime and dereferences the NULL
pointer.  This is a conditional NULL-pointer dereference / oops on an
ordinary PCM close; a narrower runtime-UAF window is theoretically
possible but was not reproduced, so this report does not claim UAF.
The handler also violates the ALSA convention that snd_pcm_action()
must be called with the PCM stream lock held.

The detailed description:

* Test environment / affected version

  Linux 7.3.0 source tree (VERSION=7, PATCHLEVEL=3, SUBLEVEL=0),
  sound/usb/usx2y/us144mkii_playback.c, us144mkii_pcm.c and
  us144mkii.h.  The driver binds the TASCAM US-144MKII (0644:8020) and
  US-144 (0644:800f).  The relevant configuration is
  CONFIG_SND_USB_US144MKII=m.

  This report is based on source inspection, plus a synthetic ALSA PCM
  model used to validate the close/detach ordering.  I have not run the
  real driver on TASCAM hardware; no real-hardware result is claimed.
  The attached POC is that synthetic-ordering model, not a TASCAM
  hardware reproducer.

* Mechanism

  1. The work is queued on feedback sync loss:

             if (tascam->feedback_synced) {
                     tascam->feedback_consecutive_errors++;
                     if (tascam->feedback_consecutive_errors >
                         FEEDBACK_SYNC_LOSS_THRESHOLD) {
                             dev_err(tascam->card->dev,
                                     "Fatal: Feedback sync lost. Stopping stream.\n");
                             schedule_work(&tascam->stop_pcm_work);
                             tascam->feedback_synced = false;
                             break;
                     }
             }

     (us144mkii_playback.c:347-356).  FEEDBACK_SYNC_LOSS_THRESHOLD is
     41 (us144mkii.h:88), so the condition is reached after 42
     consecutive feedback errors.

  2. The handler reads the pointers without a lock:

             void tascam_stop_pcm_work_handler(struct work_struct *work)
             {
                     struct tascam_card *tascam =
                             container_of(work, struct tascam_card,
                                          stop_pcm_work);

                     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);
             }

     (us144mkii_playback.c:447-456).

  3. PCM close only NULLs the pointers and does not cancel the work:

             tascam->playback_substream = NULL;   /* or capture */
             return 0;

     (us144mkii_playback.c:35-42, us144mkii_capture.c:35-42).  There
     is no cancel_work_sync(&tascam->stop_pcm_work) in either close
     callback.

  4. ALSA then detaches and frees the runtime:

             snd_pcm_drop(substream);
             ...
             substream->ops->close(substream);
             ...
             snd_pcm_detach_substream(substream);

     (sound/core/pcm_native.c:2785-2805; sound/core/pcm.c:978-1005:
     substream->runtime = NULL at 997, kfree(runtime) at 1001).

  5. snd_pcm_stop() is not safe without the stream lock.
     snd_pcm_action() is documented "Note: call with stream lock"
     (sound/core/pcm_native.c:1411-1412); snd_pcm_pre_stop() does:

             struct snd_pcm_runtime *runtime = substream->runtime;
             if (runtime->state == SNDRV_PCM_STATE_OPEN)
                     return -EBADFD;
             runtime->trigger_master = substream;

     (sound/core/pcm_native.c:1549-1558).  After detach runtime is
     NULL, so the first dereference is a NULL-pointer oops.
     snd_pcm_do_stop()/snd_pcm_post_stop() would also touch
     runtime->trigger_master and runtime->sleep/tsleep (1570-1577).

  6. Interleaving: the handler loads tascam->playback_substream (or
     capture_substream) before close clears it, is preempted, close then
     detaches and frees the runtime, and the handler proceeds into
     snd_pcm_stop() and touches the NULL/freed runtime.  The workqueue
     item does not need to overlap with an URB completion at close time;
     it only needs to be queued before close and executed after detach.
     If the handler already passed a runtime non-NULL check before
     detach, a narrower UAF is possible, but that form was not
     reproduced and is not the primary claim.

* Trigger model

  Feedback sync loss is needed: 42 consecutive feedback errors while
  playback or capture is running.  This can be produced with a USB
  traffic fault injector, a short-packet/loss injection on the feedback
  endpoint, or a programmable USB gadget emulating the device.  After
  the driver logs the feedback sync loss and queues the work, close the
  PCM stream before the work runs.  No special privilege beyond access
  to the PCM device is required; TASCAM US-144MKII/US-144 hardware (or
  an emulated equivalent) is required.

  Consequence: NULL-pointer dereference / oops in
  tascam_stop_pcm_work_handler() -> snd_pcm_stop() ->
  snd_pcm_pre_stop(); because the work is already queued, this is
  independent of the URB completion timing in Case 1.

* Reproducer (attached POC)

  Attached files:
    tascam_poc.c  - synthetic ALSA PCM kernel module (C2 mode pcm2)
    pcmtest.c     - userland exerciser
    Makefile      - builds tascam_poc.ko against the running kernel

  This POC does not drive the TASCAM device or inject feedback errors.
  It creates a synthetic ALSA PCM card whose .close() only drops the
  substream pointer, queues work that caches the substream, starts the
  stream, and lets ALSA detach the runtime before the work calls
  snd_pcm_stop().  It therefore reproduces the close/detach ordering in
  point 4 above, not the real feedback-error path.

  Build and run:
    make
    gcc -static -O2 -o pcmtest pcmtest.c
    sudo modprobe snd-pcm
    sudo insmod ./tascam_poc.ko mode=pcm2
    sudo ./pcmtest 1
    dmesg | tail -50
    sudo rmmod tascam_poc

  Expected result:
    sndusb_race: c2 call snd_pcm_stop ss=... runtime=0000000000000000
    BUG: KASAN: null-ptr-deref in snd_pcm_pre_stop+0x24/0x50
    BUG: kernel NULL pointer dereference, address: 0000000000000000
    Oops: 0000 [#1] SMP KASAN NOPTI
    RIP: snd_pcm_pre_stop+0x24/0x50

  Negative control (close cancels the work):
    sudo insmod ./tascam_poc.ko mode=pcm2 safe=1
    sudo ./pcmtest 1
  Expected: no oops/KASAN report.

  The module creates a capture PCM at card index 0; if that index is
  occupied, check /proc/asound/cards and adjust DEV in pcmtest.c.

  A minimal real-hardware KASAN test would:
    - start playback and/or capture;
    - inject 42 consecutive feedback errors (or use a gadget that
      returns short/error packets on the feedback endpoint) until the
      driver logs "Fatal: Feedback sync lost. Stopping stream.";
    - immediately close the PCM stream in a loop;
    - watch dmesg for a KASAN report with
      tascam_stop_pcm_work_handler in the stack.


* Suggested direction

  Make .close() cancel the work before returning to ALSA: clear the
  active flag and pointer under tascam->lock, kill the relevant URBs
  per-URB, then cancel_work_sync(&tascam->stop_pcm_work) (and
  stop_work/capture_work as appropriate).  cancel_work_sync() is the
  lifecycle guarantee; it waits for a running handler.

  In the handler, use snd_pcm_stop_xrun()
  (sound/core/pcm_native.c:1628-1635), which takes the PCM stream lock
  and rechecks substream->runtime.  That fixes the API-convention
  violation, but it is not a substitute for cancelling the work in
  close: snd_pcm_detach_substream() does not take the stream lock, so
  a handler that already passed the runtime check can still race with
  detach.

  I am happy to test a proposed fix.

Please keep me in Cc; I am not subscribed to the lists.


Regards,
Hanxiang Xu
xuhx@hust.edu.cn

[-- Attachment #2: tascam_poc.c --]
[-- Type: text/plain, Size: 6616 bytes --]

// SPDX-License-Identifier: GPL-2.0
/*
 * tascam_poc.c - synthetic ALSA PCM lifetime reproducer for the TASCAM
 * US-144MKII manual-review cases C1 and C2.
 *
 * This module deliberately reproduces the *lifetime patterns* of the driver,
 * not the USB device itself:
 *   pcm1 : ALSA PCM .close() drops the runtime while an asynchronous
 *          "completion" still holds a runtime pointer (C1).
 *   pcm2 : a queued work item calls snd_pcm_stop() after .close()/detach
 *          (C2).
 *
 * The ALSA card is a synthetic platform device; no TASCAM hardware or USB
 * traffic is involved.  Build and run instructions are in README.txt.
 */
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/init.h>
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/kthread.h>
#include <linux/wait.h>
#include <linux/completion.h>
#include <linux/platform_device.h>
#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>

#define DRV "sndusb_race"
#define COOKIE 0x5a5a1234deadbeefULL

static char *mode = "pcm1";
module_param(mode, charp, 0444);
static bool safe;
module_param(safe, bool, 0444);
MODULE_PARM_DESC(mode, "pcm1 or pcm2");
MODULE_PARM_DESC(safe, "take the safe cleanup path (negative control)");

/* ------------------------------------------------------------------ */
/* V1/V2 synthetic ALSA PCM                                            */
/* ------------------------------------------------------------------ */
struct race_pcm {
	struct snd_pcm_substream *substream;
	struct snd_pcm_runtime *runtime;
	struct work_struct work;
	atomic_t active;
	bool c2;
	u64 cookie;
};

static struct snd_card *race_card;
static struct platform_device *race_pdev;
static struct snd_pcm *race_pcm_dev;

static void race_pcm_work(struct work_struct *work)
{
	struct race_pcm *m = container_of(work, struct race_pcm, work);
	struct snd_pcm_runtime *rt = m->runtime;
	struct snd_pcm_substream *ss = m->substream;

	pr_info(DRV ": work start mode=%s rt=%px ss=%px cookie=%#llx\n",
		m->c2 ? "c2" : "c1", rt, ss, m->cookie);
	if (!m->c2 && !rt)
		return;
	if (m->c2 && !ss)
		return;

	/* Give the closing thread time to run .close() and detach/free. */
	msleep(120);

	if (!m->c2) {
		/*
		 * C1: the completion cached runtime before close; after
		 * snd_pcm_detach_substream() this is freed memory.
		 */
		pr_info(DRV ": c1 use rt=%px dma_area=%px\n", rt,
			READ_ONCE(rt->dma_area));
		barrier();
		(void)READ_ONCE(rt->dma_area);
	} else {
		/*
		 * C2: the work cached the substream, but detach made
		 * ss->runtime NULL; snd_pcm_stop() dereferences it.
		 */
		pr_info(DRV ": c2 call snd_pcm_stop ss=%px runtime=%px\n",
			ss, READ_ONCE(ss->runtime));
		snd_pcm_stop(ss, SNDRV_PCM_STATE_XRUN);
	}
	pr_info(DRV ": work end\n");
}

static int race_pcm_open(struct snd_pcm_substream *substream)
{
	struct snd_pcm_runtime *runtime = substream->runtime;
	struct race_pcm *m;

	m = kzalloc(sizeof(*m), GFP_KERNEL);
	if (!m)
		return -ENOMEM;
	m->substream = substream;
	m->runtime = runtime;
	m->cookie = COOKIE;
	m->c2 = !strcmp(mode, "pcm2");
	INIT_WORK(&m->work, race_pcm_work);
	atomic_set(&m->active, 0);
	runtime->private_data = m;
	runtime->hw.info = SNDRV_PCM_INFO_MMAP |
			   SNDRV_PCM_INFO_INTERLEAVED |
			   SNDRV_PCM_INFO_MMAP_VALID;
	runtime->hw.formats = SNDRV_PCM_FMTBIT_S16_LE;
	runtime->hw.rates = SNDRV_PCM_RATE_44100;
	runtime->hw.rate_min = 44100;
	runtime->hw.rate_max = 44100;
	runtime->hw.channels_min = 2;
	runtime->hw.channels_max = 2;
	runtime->hw.buffer_bytes_max = 64 * 1024;
	runtime->hw.period_bytes_min = 1024;
	runtime->hw.period_bytes_max = 4096;
	runtime->hw.periods_min = 2;
	runtime->hw.periods_max = 8;
	pr_info(DRV ": open sub=%px rt=%px mode=%s\n", substream, runtime, mode);
	return 0;
}

static int race_pcm_close(struct snd_pcm_substream *substream)
{
	struct snd_pcm_runtime *runtime = substream->runtime;
	struct race_pcm *m = runtime ? runtime->private_data : NULL;

	pr_info(DRV ": close sub=%px rt=%px m=%px safe=%d\n",
		substream, runtime, m, safe);
	if (m && safe)
		cancel_work_sync(&m->work);
	if (m) {
		m->substream = NULL;
		m->runtime = NULL;
	}
	return 0;
}

static int race_pcm_hw_params(struct snd_pcm_substream *substream,
			      struct snd_pcm_hw_params *params)
{
	return 0;
}

static int race_pcm_prepare(struct snd_pcm_substream *substream)
{
	return 0;
}

static int race_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
{
	struct snd_pcm_runtime *runtime = substream->runtime;
	struct race_pcm *m = runtime ? runtime->private_data : NULL;

	if (!m)
		return 0;
	switch (cmd) {
	case SNDRV_PCM_TRIGGER_START:
	case SNDRV_PCM_TRIGGER_RESUME:
		atomic_set(&m->active, 1);
		pr_info(DRV ": trigger start, queue work\n");
		schedule_work(&m->work);
		break;
	case SNDRV_PCM_TRIGGER_STOP:
	case SNDRV_PCM_TRIGGER_SUSPEND:
		atomic_set(&m->active, 0);
		break;
	}
	return 0;
}

static snd_pcm_uframes_t race_pcm_pointer(struct snd_pcm_substream *substream)
{
	return 0;
}

static const struct snd_pcm_ops race_pcm_ops = {
	.open = race_pcm_open,
	.close = race_pcm_close,
	.hw_params = race_pcm_hw_params,
	.prepare = race_pcm_prepare,
	.trigger = race_pcm_trigger,
	.pointer = race_pcm_pointer,
};

static int sndusb_pcm_init(void)
{
	int ret;

	race_pdev = platform_device_register_simple("sndusb_race", -1, NULL, 0);
	if (IS_ERR(race_pdev))
		return PTR_ERR(race_pdev);
	ret = snd_card_new(&race_pdev->dev, 0, "SndUsbRace", THIS_MODULE, 0,
			   &race_card);
	if (ret < 0) {
		pr_err(DRV ": snd_card_new failed: %d\n", ret);
		return ret;
	}
	ret = snd_pcm_new(race_card, "SndUsbRace", 0, 0, 1, &race_pcm_dev);
	if (ret < 0) {
		pr_err(DRV ": snd_pcm_new failed: %d\n", ret);
		snd_card_free(race_card);
		return ret;
	}
	snd_pcm_set_ops(race_pcm_dev, SNDRV_PCM_STREAM_CAPTURE, &race_pcm_ops);
	snd_pcm_lib_preallocate_pages_for_all(race_pcm_dev,
					      SNDRV_DMA_TYPE_CONTINUOUS, NULL,
					      64 * 1024, 64 * 1024);
	ret = snd_card_register(race_card);
	if (ret < 0) {
		pr_err(DRV ": snd_card_register failed: %d\n", ret);
		snd_card_free(race_card);
		return ret;
	}
	pr_info(DRV ": card registered mode=%s safe=%d\n", mode, safe);
	return 0;
}

static int __init sndusb_init(void)
{
	pr_info(DRV ": init mode=%s safe=%d\n", mode, safe);
	return sndusb_pcm_init();
}

static void __exit sndusb_exit(void)
{
	pr_info(DRV ": exit\n");
	if (race_card)
		snd_card_free(race_card);
	if (race_pdev)
		platform_device_unregister(race_pdev);
}

module_init(sndusb_init);
module_exit(sndusb_exit);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("TASCAM US-144MKII C1/C2 synthetic lifetime reproducer");

[-- Attachment #3: pcmtest.c --]
[-- Type: text/plain, Size: 3737 bytes --]

#define _GNU_SOURCE
// SPDX-License-Identifier: GPL-2.0
/*
 * pcmtest.c - guest-side exerciser for sndpci_race.ko.
 *
 * Usage:
 *   pcmtest close [iters]  - open/start/close one playback substream (V4)
 *   pcmtest link  [iters]  - link two playback substreams, close B while
 *                            the A timer callback walks the group (V5)
 */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/ioctl.h>
#include <limits.h>
#include <sched.h>
#include <sound/asound.h>

#define DEV "/dev/snd/pcmC0D0c"

static int set_hw_params(int fd)
{
	struct snd_pcm_hw_params p;
	int i, ret;

	memset(&p, 0, sizeof(p));
	for (i = 0; i <= SNDRV_PCM_HW_PARAM_LAST_MASK -
			SNDRV_PCM_HW_PARAM_FIRST_MASK; i++)
		memset(&p.masks[i], 0xff, sizeof(p.masks[i]));
	for (i = 0; i <= SNDRV_PCM_HW_PARAM_LAST_INTERVAL -
			SNDRV_PCM_HW_PARAM_FIRST_INTERVAL; i++) {
		p.intervals[i].min = 0;
		p.intervals[i].max = UINT_MAX;
		p.intervals[i].integer = 0;
	}
	p.rmask = ~0U;
	ret = ioctl(fd, SNDRV_PCM_IOCTL_HW_REFINE, &p);
	printf("HW_REFINE ret=%d errno=%d\n", ret, errno);
	if (ret < 0)
		return -1;
	printf("refined: rate=%u ch=%u fmt=%u ps=%u periods=%u bs=%u\n",
	       p.intervals[SNDRV_PCM_HW_PARAM_RATE - SNDRV_PCM_HW_PARAM_FIRST_INTERVAL].min,
	       p.intervals[SNDRV_PCM_HW_PARAM_CHANNELS - SNDRV_PCM_HW_PARAM_FIRST_INTERVAL].min,
	       __builtin_ctz(p.masks[SNDRV_PCM_HW_PARAM_FORMAT].bits[0]),
	       p.intervals[SNDRV_PCM_HW_PARAM_PERIOD_SIZE - SNDRV_PCM_HW_PARAM_FIRST_INTERVAL].min,
	       p.intervals[SNDRV_PCM_HW_PARAM_PERIODS - SNDRV_PCM_HW_PARAM_FIRST_INTERVAL].min,
	       p.intervals[SNDRV_PCM_HW_PARAM_BUFFER_SIZE - SNDRV_PCM_HW_PARAM_FIRST_INTERVAL].min);
	ret = ioctl(fd, SNDRV_PCM_IOCTL_HW_PARAMS, &p);
	if (ret < 0) {
		fprintf(stderr, "HW_PARAMS: %s\n", strerror(errno));
		return -1;
	}
	return 0;
}

static int open_stream(void)
{
	int fd = open(DEV, O_RDWR | O_NONBLOCK);

	if (fd < 0)
		fprintf(stderr, "open %s: %s\n", DEV, strerror(errno));
	return fd;
}

static int prepare_stream(int fd)
{
	if (set_hw_params(fd) < 0)
		return -1;
	if (ioctl(fd, SNDRV_PCM_IOCTL_PREPARE, 0) < 0) {
		fprintf(stderr, "PREPARE: %s\n", strerror(errno));
		return -1;
	}
	return 0;
}

static int start_stream(int fd)
{
	if (ioctl(fd, SNDRV_PCM_IOCTL_START, 0) < 0) {
		fprintf(stderr, "START: %s\n", strerror(errno));
		return -1;
	}
	return 0;
}

static int one_close(void)
{
	int fd = open_stream();

	if (fd < 0)
		return -1;
	if (prepare_stream(fd) < 0 || start_stream(fd) < 0) {
		close(fd);
		return -1;
	}
	usleep(10000);
	close(fd);
	return 0;
}

static int one_link(void)
{
	int a = open_stream();
	int b = open_stream();

	if (a < 0 || b < 0)
		return -1;
	if (prepare_stream(a) < 0 || prepare_stream(b) < 0)
		goto out;
	if (ioctl(a, SNDRV_PCM_IOCTL_LINK, b) < 0) {
		fprintf(stderr, "LINK: %s\n", strerror(errno));
		goto out;
	}
	if (start_stream(a) < 0)
		goto out;
	/* Let the A timer callback start walking the linked group. */
	usleep(1500);
	/* Close B while A's callback may be in the group loop. */
	close(b);
	b = -1;
	usleep(1500);
out:
	if (b >= 0)
		close(b);
	if (a >= 0)
		close(a);
	return 0;
}

int main(int argc, char **argv)
{
	cpu_set_t set;

	CPU_ZERO(&set);
	CPU_SET(1, &set);
	sched_setaffinity(0, sizeof(set), &set);
	int i, n = 200;

	if (argc < 2) {
		fprintf(stderr, "usage: %s close|link [iters]\n", argv[0]);
		return 2;
	}
	if (argc > 2)
		n = atoi(argv[2]);
	printf("pcmtest: mode=%s iters=%d\n", argv[1], n);
	for (i = 0; i < n; i++) {
		if (!strcmp(argv[1], "link")) {
			if (one_link() < 0)
				break;
		} else {
			if (one_close() < 0)
				break;
		}
	}
	printf("pcmtest: done i=%d\n", i);
	return 0;
}

[-- Attachment #4: Makefile --]
[-- Type: application/octet-stream, Size: 320 bytes --]

# Build against the running kernel's headers.
# For a KASAN test kernel, set KDIR to that build tree, e.g.:
#   make KDIR=/path/to/linux KASAN=1
KDIR ?= /lib/modules/$(shell uname -r)/build
PWD  := $(shell pwd)

all:
	$(MAKE) -C $(KDIR) M=$(PWD) modules

clean:
	$(MAKE) -C $(KDIR) M=$(PWD) clean

obj-m := tascam_poc.o