[PATCH 0/6] ALSA: sound/core: fix multiple data races and bugs

To: Takashi Iwai <[email protected]>, Mark Brown <[email protected]>
Cc: [email protected]
Cc: [email protected]

Hi Takashi, Mark,

This is my first patch series to the Linux kernel.  I am CCing the
security team as recommended by the docs for initial reports.

This series fixes 6 bugs found via code review and KCSAN dynamic
analysis in the ALSA sound/core subsystem.  All patches are against
current HEAD (4edcdefd4083).

Patch 1 is confirmed with KCSAN.  The remaining patches address bugs
found during analysis of the same code paths.

This work was AI-assisted.  Per the kernel documentation on reporting
security bugs, it is sent to the public mailing list.  All
reproducers have been tested on arm64 QEMU.

Summary:

  [1/6] ALSA: compress: remove illegal state mutation in poll()
    snd_compr_poll() writes runtime->state = SETUP when it sees
    DRAINING.  This races with snd_compress_wait_for_drain() which
    reads runtime->state concurrently.  KCSAN confirms:

      BUG: KCSAN: data-race in snd_compr_poll / snd_compress_wait_for_drain

      write to 0xffff0000c75c8200 of 4 bytes by task 282 on cpu 3:
       snd_compr_poll+0x29c/0x2c8
       do_sys_poll+0x2b0/0x628

      read to 0xffff0000c75c8200 of 4 bytes by task 279 on cpu 1:
       snd_compress_wait_for_drain+0xa4/0x270
       snd_compr_ioctl+0x1ba0/0x1bc8

      value changed: 0x00000005 -> 0x00000001

    poll() is a query operation and must not mutate state.  The
    DRAINING->SETUP transition is already handled by drivers via
    snd_compr_drain_notify().

  [2/6] ALSA: compress: fix buffer leak on set_params driver failure
    If ops->set_params() fails after buffer allocation, the buffer
    leaks on retry because runtime->buffer was already overwritten.

  [3/6] ALSA: timer: fix lockless tu->tread read in read()
    snd_timer_user_read() reads tu->tread without ioctl_lock to
    determine entry size, then re-reads it under lock for the copy
    format.  A concurrent TREAD ioctl can change the format between
    reads, causing a size/format mismatch.

  [4/6] ALSA: timer: copy queue entries under lock before copy_to_user
    After dequeuing under qlock, read() drops the lock and calls
    copy_to_user() directly from the queue slot.  Timer callbacks
    can overwrite that slot via queue wraparound during the copy.
    Same bug class as CVE-2017-1000380.

  [5/6] ALSA: pcm: use locked state read in snd_pcm_drop()
    snd_pcm_drop() reads runtime->state without the stream lock.
    Commit 7bc02ab446d3 fixed this pattern in read/write paths
    but missed drop().

  [6/6] ALSA: rawmidi: propagate resize_runtime_buffer() error
    snd_rawmidi_input_params() ignores the return value from
    resize_runtime_buffer() and unconditionally returns 0.

KCSAN reproducer for patch 1:

  The race is triggered by concurrent poll() and DRAIN ioctl on a
  compress offload stream.  A minimal reproducer uses four threads:

    Thread 1: DRAIN ioctl loop
    Thread 2: PARTIAL_DRAIN ioctl loop
    Thread 3: STOP ioctl loop (random delay)
    Thread 4: poll() loop

  The main thread repeatedly sets up and starts the stream.  With
  CONFIG_KCSAN=y and CONFIG_KCSAN_STRICT=y, the race is flagged
  within ~2000 iterations.

  Any compress offload driver (hardware or virtual) exposes the
  race since the bug is in the framework, not driver code.

  Tested on arm64 QEMU, kernel 7.1.0-rc7+ (4edcdefd4083).

Thanks,
Omer