[PATCH] ALSA: pcm: Keep linked drain wait entries safe across unlink

[PATCH] ALSA: pcm: Keep linked drain wait entries safe across unlink

[Cen Zhang]

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 <zzzccc427@gmail.com>

---

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;

Re: [PATCH] ALSA: pcm: Keep linked drain wait entries safe across unlink

[Takashi Iwai]

On Thu, 15 Oct 2026 19:10:29 +0200,
Cen Zhang wrote:
> 
> 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 <zzzccc427@gmail.com>

This doesn't look needed / fitting for the latest code?


Takashi

Re: [PATCH] ALSA: pcm: Keep linked drain wait entries safe across unlink

[Cen Zhang]

Hi Takashi,

Takashi Iwai <tiwai@suse.de> 于2026年10月16日周五 01:57写道：
>
> On Thu, 15 Oct 2026 19:10:29 +0200,
> Cen Zhang wrote:
> >
> > 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 <zzzccc427@gmail.com>
>
> This doesn't look needed / fitting for the latest code?
>
>
> Takashi

You're right. Please disregard it. Thanks for pointing this out.

Best regards,
Cen Zhang