[PATCH] ALSA: pcm: Drain proc writers before freeing preallocated buffers

[PATCH] ALSA: pcm: Drain proc writers before freeing preallocated buffers

[Cen Zhang]

The prealloc proc write callback changes a substream's DMA buffer under
pcm->open_mutex. Final PCM cleanup frees that buffer without the mutex,
before removing the stream proc entries. A pending callback can therefore
race the free or allocate a replacement after the final buffer cleanup.

This is reachable through codec-only HDA sysfs unbind while the card
remains registered. With CONFIG_SND_VERBOSE_PROCFS, HDA creates writable
prealloc files with a nonzero maximum. Buffer a valid size different from
the current preallocation and leave the file open, without a PCM runtime.
Cleanup reaches snd_pcm_free() through hda_codec_driver_remove(),
snd_hda_codec_cleanup_for_unbind(), codec_release_pcms() and
snd_device_free(). The proc file pins the card module, not a codec PCM
reference, so it does not prevent this component cleanup.

The text write is applied at file release, rather than by write(2).
A close in the gap can leave a replacement buffer behind:

  PCM component cleanup           prealloc file close
  ---------------------           -------------------
  snd_pcm_free()
    free preallocated buffers
                                  snd_info_text_entry_release()
                                    snd_pcm_lib_preallocate_proc_write()
                                      allocate replacement buffer
                                      replace dma_buffer
    snd_pcm_free_stream()
      remove and drain proc files
      free substream

The same late allocation can occur without a concurrent close: the later
proc rundown itself invokes the buffered write callback. In either case,
there is no final buffer free after the callback installs its replacement.

Move each substream's preallocation free into snd_pcm_free_stream(),
after snd_pcm_stream_proc_done() has removed the proc entries and drained
their callbacks. Keep rundown outside open_mutex, which the callback needs
to finish. Both stream directions and construction-error cleanup still
free their preallocations before destroying the substreams.

Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 sound/core/pcm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 1e4e0ecb3dbd..ce577c303a1c 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -825,6 +825,7 @@ static void snd_pcm_free_stream(struct snd_pcm_str * pstr)
 	substream = pstr->substream;
 	while (substream) {
 		substream_next = substream->next;
+		snd_pcm_lib_preallocate_free(substream);
 		snd_pcm_timer_done(substream);
 		kfree(substream);
 		substream = substream_next;
@@ -861,7 +862,6 @@ static int snd_pcm_free(struct snd_pcm *pcm)
 	}
 	if (pcm->private_free)
 		pcm->private_free(pcm);
-	snd_pcm_lib_preallocate_free_for_all(pcm);
 	snd_pcm_free_stream(&pcm->streams[SNDRV_PCM_STREAM_PLAYBACK]);
 	snd_pcm_free_stream(&pcm->streams[SNDRV_PCM_STREAM_CAPTURE]);
 	kfree(pcm);
-- 
2.43.0

Re: [PATCH] ALSA: pcm: Drain proc writers before freeing preallocated buffers

[Takashi Iwai]

On Tue, 20 Oct 2026 04:29:24 +0200,
Cen Zhang wrote:
> 
> The prealloc proc write callback changes a substream's DMA buffer under
> pcm->open_mutex. Final PCM cleanup frees that buffer without the mutex,
> before removing the stream proc entries. A pending callback can therefore
> race the free or allocate a replacement after the final buffer cleanup.
> 
> This is reachable through codec-only HDA sysfs unbind while the card
> remains registered. With CONFIG_SND_VERBOSE_PROCFS, HDA creates writable
> prealloc files with a nonzero maximum. Buffer a valid size different from
> the current preallocation and leave the file open, without a PCM runtime.
> Cleanup reaches snd_pcm_free() through hda_codec_driver_remove(),
> snd_hda_codec_cleanup_for_unbind(), codec_release_pcms() and
> snd_device_free(). The proc file pins the card module, not a codec PCM
> reference, so it does not prevent this component cleanup.
> 
> The text write is applied at file release, rather than by write(2).
> A close in the gap can leave a replacement buffer behind:
> 
>   PCM component cleanup           prealloc file close
>   ---------------------           -------------------
>   snd_pcm_free()
>     free preallocated buffers
>                                   snd_info_text_entry_release()
>                                     snd_pcm_lib_preallocate_proc_write()
>                                       allocate replacement buffer
>                                       replace dma_buffer
>     snd_pcm_free_stream()
>       remove and drain proc files
>       free substream
> 
> The same late allocation can occur without a concurrent close: the later
> proc rundown itself invokes the buffered write callback. In either case,
> there is no final buffer free after the callback installs its replacement.
> 
> Move each substream's preallocation free into snd_pcm_free_stream(),
> after snd_pcm_stream_proc_done() has removed the proc entries and drained
> their callbacks. Keep rundown outside open_mutex, which the callback needs
> to finish. Both stream directions and construction-error cleanup still
> free their preallocations before destroying the substreams.
> 
> Assisted-by: LLM
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
>  sound/core/pcm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/core/pcm.c b/sound/core/pcm.c
> index 1e4e0ecb3dbd..ce577c303a1c 100644
> --- a/sound/core/pcm.c
> +++ b/sound/core/pcm.c
> @@ -825,6 +825,7 @@ static void snd_pcm_free_stream(struct snd_pcm_str * pstr)
>  	substream = pstr->substream;
>  	while (substream) {
>  		substream_next = substream->next;
> +		snd_pcm_lib_preallocate_free(substream);
>  		snd_pcm_timer_done(substream);
>  		kfree(substream);
>  		substream = substream_next;
> @@ -861,7 +862,6 @@ static int snd_pcm_free(struct snd_pcm *pcm)
>  	}
>  	if (pcm->private_free)
>  		pcm->private_free(pcm);
> -	snd_pcm_lib_preallocate_free_for_all(pcm);
>  	snd_pcm_free_stream(&pcm->streams[SNDRV_PCM_STREAM_PLAYBACK]);
>  	snd_pcm_free_stream(&pcm->streams[SNDRV_PCM_STREAM_CAPTURE]);
>  	kfree(pcm);

I believe a better fix is to handle the disconnection of procfs more
properly.  The hda codec driver calls snd_device_disconnect() for each
pcm, but in the PCM side, the procfs removal relies on the card level
disconnection (snd_info_card_disconnect()).

That is, something like below works instead?


thanks,

Takashi

--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -1119,6 +1119,7 @@ static int snd_pcm_dev_disconnect(struct snd_device *device)
 	for (cidx = 0; cidx < 2; cidx++) {
 		if (pcm->streams[cidx].dev)
 			snd_unregister_device(pcm->streams[cidx].dev);
+		snd_pcm_stream_proc_done(&pcm->streams[cidx]);
 		free_chmap(&pcm->streams[cidx]);
 	}
 	return 0;