Re: [PATCH v32 12/20] ALSA: virtio: use request-owned capture and serialize recovery

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] ALSA: virtio: premature capture drain teardown in
  virtsnd_pcm_sync_stop()
--

commit 9ad9852068461024197debdd2bd36f84e60f028e
Author: Joonho Lee <mynameisjune111@gmail.com>

ALSA: virtio: use request-owned capture and serialize recovery

This commit switches VirtIO ALSA capture to request-owned buffers with
in-order completion commits and serializes PCM control transitions, reset
handling, and error recovery across substream generations.

Link: https://lore.kernel.org/linux-sound/sashiko-outbox-210240@kernel.org/

> diff --git a/sound/virtio/virtio_pcm_ops.c b/sound/virtio/virtio_pcm_ops.c
> index a5a682534acf7..442169a516676 100644
> --- a/sound/virtio/virtio_pcm_ops.c
> +++ b/sound/virtio/virtio_pcm_ops.c

[ ... ]

> +static int virtsnd_pcm_sync_stop(struct snd_pcm_substream *substream)
> +{
> +	struct virtio_pcm_substream *vss = snd_pcm_substream_chip(substream);
> +	bool teardown_pending;
> +
> +	guard(mutex)(&vss->lifecycle_lock);
> +	scoped_guard(pcm_stream_lock_irqsave, substream) {
> +		snd_pcm_state_t state = substream->runtime->state;
> +
> +		/*
> +		 * Trigger holds the PCM lock through its control requests. Wait
> +		 * for it, then leave a replacement active generation alone.
> +		 * lifecycle_lock keeps prepare from replacing an inactive one
> +		 * until sync_stop_locked has finished its teardown.
> +		 */
> +		guard(spinlock_irqsave)(&vss->lock);
> +		teardown_pending = vss->prepared || vss->release_required ||
> +			vss->msg_count || vss->device_running || vss->stopped;
> +		if (state == SNDRV_PCM_STATE_RUNNING ||
> +		    state == SNDRV_PCM_STATE_PAUSED ||
> +		    state == SNDRV_PCM_STATE_PREPARED ||
> +		    (state == SNDRV_PCM_STATE_DRAINING &&
> +		     substream->stream == SNDRV_PCM_STREAM_PLAYBACK)) {
> +			/* Core may consume this marker before a later inactive teardown. */
> +			if (teardown_pending)
> +				substream->runtime->stop_operating = true;
> +			return 0;
> +		}

[Severity: Medium]
Can a delayed virtsnd_pcm_sync_stop() tear down a replacement capture
generation while it is in SNDRV_PCM_STATE_DRAINING?

snd_pcm_drop() unlocks the PCM stream lock before calling
snd_pcm_sync_stop().  If another thread prepares, starts, and drains a
capture substream with unread frames before the first thread acquires
vss->lifecycle_lock, snd_pcm_drain() leaves the substream in
SNDRV_PCM_STATE_DRAINING with vss->prepared set so userspace can read the
remaining buffered frames and in-flight STOP tail completions:

CPU0                                  CPU1
snd_pcm_drop()
  runtime->stop_operating = true
  snd_pcm_stream_unlock_irq()
  snd_pcm_sync_stop()
    runtime->stop_operating = false
                                      snd_pcm_prepare()
                                        virtsnd_pcm_prepare_locked()
                                          virtsnd_pcm_sync_stop_locked()
                                          ...
                                          vss->prepared = true
                                      snd_pcm_start()
                                      snd_pcm_drain()
                                        /* state = SNDRV_PCM_STATE_DRAINING */
    virtsnd_pcm_sync_stop()
      /* DRAINING capture not skipped */
      virtsnd_pcm_sync_stop_locked()
        vss->prepared = false         <-- aborts active capture drain

Because the active-generation check only exempts SNDRV_PCM_STATE_DRAINING
for playback, virtsnd_pcm_sync_stop_locked() sends VIRTIO_SND_R_PCM_RELEASE,
disables vss->elapsed_period, and clears vss->prepared.  Subsequent reads
and pointer updates during capture drain then see !vss->prepared in
virtsnd_pcm_capture_ack() and virtsnd_pcm_cp_pointer(), failing with -EBADFD
or SNDRV_PCM_POS_XRUN and discarding the unread capture data.

Allowing SNDRV_PCM_STATE_DRAINING for capture in the active-generation check
as well would keep the draining generation intact until its final teardown.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-fix-virtio-snd-recovery-final-v32-0-4e95979e312c@gmail.com?part=12