mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ALSA: pcm: Serialize snd_pcm_suspend_all() with open_mutex
@ 2026-03-20 17:28 Cássio Gabriel
  2026-03-27 11:38 ` Takashi Iwai
  0 siblings, 1 reply; 3+ messages in thread
From: Cássio Gabriel @ 2026-03-20 17:28 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, linux-sound, linux-kernel, Cássio Gabriel

snd_pcm_suspend_all() walks all PCM substreams and uses a lockless
runtime check to skip closed streams. It then calls snd_pcm_suspend()
for each remaining substream and finally runs snd_pcm_sync_stop() in a
second pass.

The runtime lifetime is still controlled by pcm->open_mutex in the
open/release path. That means a concurrent close can clear or free
substream->runtime after the initial check in snd_pcm_suspend_all(),
leaving the later suspend or sync-stop path to dereference a stale or
NULL runtime pointer.

Serialize snd_pcm_suspend_all() with pcm->open_mutex so the runtime
pointer stays stable across both loops. This matches the existing PCM
runtime lifetime rule already used by other core paths that access
substream->runtime outside the stream lock.

Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
---
 sound/core/pcm_native.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 674b50c7c5f6..ebcb62603d49 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1792,8 +1792,9 @@ int snd_pcm_suspend_all(struct snd_pcm *pcm)
 	if (! pcm)
 		return 0;
 
+	guard(mutex)(&pcm->open_mutex);
+
 	for_each_pcm_substream(pcm, stream, substream) {
-		/* FIXME: the open/close code should lock this as well */
 		if (!substream->runtime)
 			continue;
 

---
base-commit: b3c48fa1fb397b490101785ddd87caf2e5513a66
change-id: 20260320-alsa-pcm-suspend-open-close-lock-56907d40df44

Best regards,
-- 
Cássio Gabriel <cassiogabrielcontato@gmail.com>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ALSA: pcm: Serialize snd_pcm_suspend_all() with open_mutex
  2026-03-20 17:28 [PATCH] ALSA: pcm: Serialize snd_pcm_suspend_all() with open_mutex Cássio Gabriel
@ 2026-03-27 11:38 ` Takashi Iwai
  2026-03-27 12:28   ` Cássio Gabriel Monteiro Pires
  0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2026-03-27 11:38 UTC (permalink / raw)
  To: Cássio Gabriel
  Cc: Takashi Iwai, Jaroslav Kysela, linux-sound, linux-kernel

On Fri, 20 Mar 2026 18:28:35 +0100,
Cássio Gabriel wrote:
> 
> snd_pcm_suspend_all() walks all PCM substreams and uses a lockless
> runtime check to skip closed streams. It then calls snd_pcm_suspend()
> for each remaining substream and finally runs snd_pcm_sync_stop() in a
> second pass.
> 
> The runtime lifetime is still controlled by pcm->open_mutex in the
> open/release path. That means a concurrent close can clear or free
> substream->runtime after the initial check in snd_pcm_suspend_all(),
> leaving the later suspend or sync-stop path to dereference a stale or
> NULL runtime pointer.
> 
> Serialize snd_pcm_suspend_all() with pcm->open_mutex so the runtime
> pointer stays stable across both loops. This matches the existing PCM
> runtime lifetime rule already used by other core paths that access
> substream->runtime outside the stream lock.
> 
> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>

I guess it's safe to take open_mutex from this function, but it's
still worth to mention it in the function description.
Could you update the description of snd_pcm_suspend_all(), too?


thanks,

Takashi

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ALSA: pcm: Serialize snd_pcm_suspend_all() with open_mutex
  2026-03-27 11:38 ` Takashi Iwai
@ 2026-03-27 12:28   ` Cássio Gabriel Monteiro Pires
  0 siblings, 0 replies; 3+ messages in thread
From: Cássio Gabriel Monteiro Pires @ 2026-03-27 12:28 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: Takashi Iwai, Jaroslav Kysela, linux-sound, linux-kernel

On 3/27/26 08:38, Takashi Iwai wrote:
>>  walks all PCM substreams and uses a lockless
>> runtime check to skip closed streams. It then calls snd_pcm_suspend()
>> for each remaining substream and finally runs snd_pcm_sync_stop() in a
>> second pass.
>>
>> The runtime lifetime is still controlled by pcm->open_mutex in the
>> open/release path. That means a concurrent close can clear or free
>> substream->runtime after the initial check in snd_pcm_suspend_all(),
>> leaving the later suspend or sync-stop path to dereference a stale or
>> NULL runtime pointer.
>>
>> Serialize snd_pcm_suspend_all() with pcm->open_mutex so the runtime
>> pointer stays stable across both loops. This matches the existing PCM
>> runtime lifetime rule already used by other core paths that access
>> substream->runtime outside the stream lock.
>>
>> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
> I guess it's safe to take open_mutex from this function, but it's
> still worth to mention it in the function description.
> Could you update the description of snd_pcm_suspend_all(), too?
> 
Yes, I'll do it and send the v2 patch.
-- 
Thanks,
Cássio


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-27 12:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-20 17:28 [PATCH] ALSA: pcm: Serialize snd_pcm_suspend_all() with open_mutex Cássio Gabriel
2026-03-27 11:38 ` Takashi Iwai
2026-03-27 12:28   ` Cássio Gabriel Monteiro Pires

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®