snd_pcm_drain() queues an on-stack wait entry on a linked playback peer's runtime->sleep. After sleeping, it calls finish_wait() only if that runtime is still found in the caller's group, since the peer may otherwise have been freed. Unlink wakes the group members before removing them, but the generic autoremove_wake_function() removes an entry only when the wake succeeds. If a signal or timeout has already made the drainer runnable, the entry can remain queued after unlink. With unlink performed before DROP at final close, the following ordering is possible: Drainer on A Final close of linked B ------------ ----------------------- Queue stack wait on B->sleep Signal wakes A: TASK_RUNNING snd_pcm_unlink(B) Wake returns 0; wait stays queued Remove B from the group Cannot find B; skip finish_wait() Return due to the pending signal snd_pcm_drop(B) Wake traverses the stale wait Use a drain-specific wake function that removes the entry under the waitqueue lock even when the task is already runnable. Preserve the ordinary wake result. This lets unlink detach the stack entry before changing membership, while keeping the runtime check around finish_wait() so drain does not access a freed peer. Also hold the existing group lock from selecting the peer through reading its runtime fields and queuing the wait. Releasing it before registration lets unlink perform its wake before the entry is queued, and can let the peer runtime be freed before it is accessed. Release the group lock after registration and on the no-peer exit path. Fixes: f57f3df03a8e ("ALSA: pcm: More fine-grained PCM link locking") Cc: stable@vger.kernel.org Signed-off-by: Cen Zhang --- diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 6c379068b54ca3bc254605dbe2fc2a27bead5f0e..e53fc8316a96f66f35f7f7db97a7de131804d2c0 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2177,6 +2177,16 @@ static const struct action_ops snd_pcm_action_drain_init = { .post_action = snd_pcm_post_drain_init }; +static int snd_pcm_drain_wake_function(wait_queue_entry_t *wait, + unsigned int mode, int flags, void *key) +{ + int ret = default_wake_function(wait, mode, flags, key); + + /* Unlink must remove the entry even if a signal already woke the task. */ + list_del_init_careful(&wait->entry); + return ret; +} + /* * Drain the stream(s). * When the substream is linked, sync until the draining of all playback streams @@ -2191,7 +2201,7 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream, struct snd_pcm_runtime *runtime; struct snd_pcm_substream *s; struct snd_pcm_group *group; - wait_queue_entry_t wait; + DEFINE_WAIT_FUNC(wait, snd_pcm_drain_wake_function); int result = 0; int nonblock = 0; @@ -2246,20 +2256,20 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream, break; } } - snd_pcm_group_unref(group, substream); - if (!to_check) + if (!to_check) { + snd_pcm_group_unref(group, substream); break; /* all drained */ + } /* - * Cache the runtime fields needed after unlock. - * A concurrent close() on the linked stream may free - * its runtime via snd_pcm_detach_substream() once we - * release the stream lock below. + * Keep the group locked until the wait is queued so unlink cannot + * wake the queue and free the peer runtime before we get there. + * Cache the fields needed after releasing the locks. */ drain_no_period_wakeup = to_check->no_period_wakeup; drain_rate = to_check->rate; drain_bufsz = to_check->buffer_size; - init_wait_entry(&wait, 0); prepare_to_wait(&to_check->sleep, &wait, TASK_INTERRUPTIBLE); + snd_pcm_group_unref(group, substream); snd_pcm_stream_unlock_irq(substream); if (drain_no_period_wakeup) tout = MAX_SCHEDULE_TIMEOUT;