From 66f2d19f8116e16898f8d82e28573a384ddc430d Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Jan 2020 20:59:07 +0100 Subject: [PATCH] ALSA: pcm: Fix memory leak at closing a stream without hw_free ALSA PCM core recently introduced a new managed PCM buffer allocation mode that does allocate / free automatically at hw_params and hw_free. However, it overlooked the code path directly calling hw_free PCM ops at releasing the PCM substream, and it may result in a memory leak as spotted by syzkaller when no buffer preallocation is used (e.g. vmalloc buffer). This patch papers over it with a slight refactoring. The hw_free ops call and relevant tasks are unified in a new helper function, and call it from both places. Fixes: 0dba808eae26 ("ALSA: pcm: Introduce managed buffer allocation mode") Reported-by: syzbot+30edd0f34bfcdc548ac4@syzkaller.appspotmail.com Cc: Link: https://lore.kernel.org/r/20200129195907.12197-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index bb23f5066654..4ac42ee1238c 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -786,10 +786,22 @@ static int snd_pcm_hw_params_user(struct snd_pcm_substream *substream, return err; } +static int do_hw_free(struct snd_pcm_substream *substream) +{ + int result = 0; + + snd_pcm_sync_stop(substream); + if (substream->ops->hw_free) + result = substream->ops->hw_free(substream); + if (substream->managed_buffer_alloc) + snd_pcm_lib_free_pages(substream); + return result; +} + static int snd_pcm_hw_free(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime; - int result = 0; + int result; if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; @@ -806,11 +818,7 @@ static int snd_pcm_hw_free(struct snd_pcm_substream *substream) snd_pcm_stream_unlock_irq(substream); if (atomic_read(&substream->mmap_count)) return -EBADFD; - snd_pcm_sync_stop(substream); - if (substream->ops->hw_free) - result = substream->ops->hw_free(substream); - if (substream->managed_buffer_alloc) - snd_pcm_lib_free_pages(substream); + result = do_hw_free(substream); snd_pcm_set_state(substream, SNDRV_PCM_STATE_OPEN); pm_qos_remove_request(&substream->latency_pm_qos_req); return result; @@ -2529,9 +2537,7 @@ void snd_pcm_release_substream(struct snd_pcm_substream *substream) snd_pcm_drop(substream); if (substream->hw_opened) { - if (substream->ops->hw_free && - substream->runtime->status->state != SNDRV_PCM_STATE_OPEN) - substream->ops->hw_free(substream); + do_hw_free(substream); substream->ops->close(substream); substream->hw_opened = 0; }