Skip to contentSkip to conversation

FAILED: patch "[PATCH] ALSA: pcm: fix use-after-free on linked stream runtime in" failed to apply to 5.10-stable tree

PATCH New [email protected] 2 participants 3 messages Mar 16

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.10.y
git checkout FETCH_HEAD
git cherry-pick -x 9b1dbd69ba6f8f8c69bc7b77c2ce3b9c6ed05ba6
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026031627-certify-copied-e4eb@gregkh' --subject-prefix 'PATCH 5.10.y' HEAD^..

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 9b1dbd69ba6f8f8c69bc7b77c2ce3b9c6ed05ba6 Mon Sep 17 00:00:00 2001
From: Mehul Rao <[email protected]>
Date: Thu, 5 Mar 2026 14:35:07 -0500
Subject: [PATCH] ALSA: pcm: fix use-after-free on linked stream runtime in
snd_pcm_drain()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In the drain loop, the local variable 'runtime' is reassigned to a linked stream's runtime (runtime = s->runtime at line 2157). After releasing the stream lock at line 2169, the code accesses runtime->no_period_wakeup, runtime->rate, and runtime->buffer_size (lines 2170-2178) — all referencing the linked stream's runtime without any lock or refcount protecting its lifetime.

A concurrent close() on the linked stream's fd triggers snd_pcm_release_substream() → snd_pcm_drop() → pcm_release_private() → snd_pcm_unlink() → snd_pcm_detach_substream() → kfree(runtime). No synchronization prevents kfree(runtime) from completing while the drain path dereferences the stale pointer.

Fix by caching the needed runtime fields (no_period_wakeup, rate, buffer_size) into local variables while still holding the stream lock, and using the cached values after the lock is released.

Fixes f2b3614cefb6 ("ALSA: PCM - Don't check DMA time-out too shortly")
sound/core/pcm_native.c +16 -3
index 67cf6a0e17ba..5a64453da728 100644
@@ -2144,6 +2144,10 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
 	for (;;) {
 		long tout;
 		struct snd_pcm_runtime *to_check;
+		unsigned int drain_rate;
+		snd_pcm_uframes_t drain_bufsz;
+		bool drain_no_period_wakeup;
+
 		if (signal_pending(current)) {
 			result = -ERESTARTSYS;
 			break;
@@ -2163,16 +2167,25 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
 		snd_pcm_group_unref(group, substream);
 		if (!to_check)
 			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.
+		 */
+		drain_no_period_wakeup = to_check->no_period_wakeup;
+		drain_rate = to_check->rate;
+		drain_bufsz = to_check->buffer_size;
 		init_waitqueue_entry(&wait, current);
 		set_current_state(TASK_INTERRUPTIBLE);
 		add_wait_queue(&to_check->sleep, &wait);
 		snd_pcm_stream_unlock_irq(substream);
-		if (runtime->no_period_wakeup)
+		if (drain_no_period_wakeup)
 			tout = MAX_SCHEDULE_TIMEOUT;
 		else {
 			tout = 100;
-			if (runtime->rate) {
-				long t = runtime->buffer_size * 1100 / runtime->rate;
+			if (drain_rate) {
+				long t = drain_bufsz * 1100 / drain_rate;
 				tout = max(t, tout);
 			}
 			tout = msecs_to_jiffies(tout);
maintainer
#1 lore raw

PATCH 1/2 ALSA: pcm: fix wait_time calculations

From: Oswald Buddenhagen <[email protected]>

[ Upstream commit 3ed2b549b39f57239aad50a255ece353997183fd ]

... in wait_for_avail() and snd_pcm_drain().

t was calculated in seconds, so it would be pretty much always zero, to be subsequently de-facto ignored due to being max(t, 10)'d. And then it (i.e., 10) would be treated as secs, which doesn't seem right.

However, fixing it to properly calculate msecs would potentially cause timeouts when using twice the period size for the default timeout (which seems reasonable to me), so instead use the buffer size plus 10 percent to be on the safe side ... but that still seems insufficient, presumably because the hardware typically needs a moment to fire up. To compensate for this, we up the minimal timeout to 100ms, which is still two orders of magnitude less than the bogus minimum.

substream->wait_time was also misinterpreted as jiffies, despite being documented as being in msecs. Only the soc/sof driver sets it - to 500, which looks very much like msecs were intended.

Speaking of which, shouldn't snd_pcm_drain() also use substream-> wait_time?

As a drive-by, make the debug messages on timeout less confusing.

Stable-dep-of: 9b1dbd69ba6f ("ALSA: pcm: fix use-after-free on linked stream runtime in snd_pcm_drain()")

---
 sound/core/pcm_lib.c    | 11 +++++------
 sound/core/pcm_native.c |  8 ++++----
 2 files changed, 9 insertions(+), 10 deletions(-)
sound/core/pcm_lib.c +5 -6
index 289f52af15b96..c28c91d4e7051 100644
@@ -1843,15 +1843,14 @@ static int wait_for_avail(struct snd_pcm_substream *substream,
 		if (substream->wait_time) {
 			wait_time = substream->wait_time;
 		} else {
-			wait_time = 10;
+			wait_time = 100;
 
 			if (runtime->rate) {
-				long t = runtime->period_size * 2 /
-					 runtime->rate;
+				long t = runtime->buffer_size * 1100 / runtime->rate;
 				wait_time = max(t, wait_time);
 			}
-			wait_time = msecs_to_jiffies(wait_time * 1000);
 		}
+		wait_time = msecs_to_jiffies(wait_time);
 	}
 
 	for (;;) {
@@ -1899,8 +1898,8 @@ static int wait_for_avail(struct snd_pcm_substream *substream,
 		}
 		if (!tout) {
 			pcm_dbg(substream->pcm,
-				"%s write error (DMA or IRQ trouble?)\n",
-				is_playback ? "playback" : "capture");
+				"%s timeout (DMA or IRQ trouble?)\n",
+				is_playback ? "playback write" : "capture read");
 			err = -EIO;
 			break;
 		}
sound/core/pcm_native.c +4 -4
index a6a3e8909be75..49bd053674bdb 100644
@@ -2155,12 +2155,12 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
 		if (runtime->no_period_wakeup)
 			tout = MAX_SCHEDULE_TIMEOUT;
 		else {
-			tout = 10;
+			tout = 100;
 			if (runtime->rate) {
-				long t = runtime->period_size * 2 / runtime->rate;
+				long t = runtime->buffer_size * 1100 / runtime->rate;
 				tout = max(t, tout);
 			}
-			tout = msecs_to_jiffies(tout * 1000);
+			tout = msecs_to_jiffies(tout);
 		}
 		tout = schedule_timeout(tout);
 
@@ -2183,7 +2183,7 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
 				result = -ESTRPIPE;
 			else {
 				dev_dbg(substream->pcm->card->dev,
-					"playback drain error (DMA or IRQ trouble?)\n");
+					"playback drain timeout (DMA or IRQ trouble?)\n");
 				snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP);
 				result = -EIO;
 			}
Signature · 1 line
2.51.0
maintainer
#2 lore raw

PATCH 2/2 ALSA: pcm: fix use-after-free on linked stream runtime in snd_pcm_drain()

From: Mehul Rao <[email protected]>

[ Upstream commit 9b1dbd69ba6f8f8c69bc7b77c2ce3b9c6ed05ba6 ]

In the drain loop, the local variable 'runtime' is reassigned to a linked stream's runtime (runtime = s->runtime at line 2157). After releasing the stream lock at line 2169, the code accesses runtime->no_period_wakeup, runtime->rate, and runtime->buffer_size (lines 2170-2178) — all referencing the linked stream's runtime without any lock or refcount protecting its lifetime.

A concurrent close() on the linked stream's fd triggers snd_pcm_release_substream() → snd_pcm_drop() → pcm_release_private() → snd_pcm_unlink() → snd_pcm_detach_substream() → kfree(runtime). No synchronization prevents kfree(runtime) from completing while the drain path dereferences the stale pointer.

Fix by caching the needed runtime fields (no_period_wakeup, rate, buffer_size) into local variables while still holding the stream lock, and using the cached values after the lock is released.

Fixes f2b3614cefb6 ("ALSA: PCM - Don't check DMA time-out too shortly")
---
 sound/core/pcm_native.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)
sound/core/pcm_native.c +16 -3
index 49bd053674bdb..09b4ad414ffbe 100644
@@ -2129,6 +2129,10 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
 	for (;;) {
 		long tout;
 		struct snd_pcm_runtime *to_check;
+		unsigned int drain_rate;
+		snd_pcm_uframes_t drain_bufsz;
+		bool drain_no_period_wakeup;
+
 		if (signal_pending(current)) {
 			result = -ERESTARTSYS;
 			break;
@@ -2148,16 +2152,25 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
 		snd_pcm_group_unref(group, substream);
 		if (!to_check)
 			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.
+		 */
+		drain_no_period_wakeup = to_check->no_period_wakeup;
+		drain_rate = to_check->rate;
+		drain_bufsz = to_check->buffer_size;
 		init_waitqueue_entry(&wait, current);
 		set_current_state(TASK_INTERRUPTIBLE);
 		add_wait_queue(&to_check->sleep, &wait);
 		snd_pcm_stream_unlock_irq(substream);
-		if (runtime->no_period_wakeup)
+		if (drain_no_period_wakeup)
 			tout = MAX_SCHEDULE_TIMEOUT;
 		else {
 			tout = 100;
-			if (runtime->rate) {
-				long t = runtime->buffer_size * 1100 / runtime->rate;
+			if (drain_rate) {
+				long t = drain_bufsz * 1100 / drain_rate;
 				tout = max(t, tout);
 			}
 			tout = msecs_to_jiffies(tout);
Signature · 1 line
2.51.0