Re: [PATCH 1/4] ALSA: compress: fix buffer leak on repeated SET_PARAMS

[Vinod Koul <vkoul@kernel.org>]

On 07-10-26, 16:25, Peter Ujfalusi wrote:
> snd_compr_allocate_buffer() unconditionally overwrites
> stream->runtime->buffer with a freshly kmalloc'd buffer whenever the
> driver has no ops->copy and no preallocated dma_buffer_p. SET_PARAMS
> is permitted repeatedly while the stream is in the OPEN state, so a
> local process can loop SNDRV_COMPRESS_SET_PARAMS and leak the
> previous buffer on every call, exhausting kernel memory.


Logically this looks right but then I dont think we have or we would
want a case where we are allowing buffer changes on the fly. Set params
on the fly would set some params like code properties etc and not code
type or buffer ones...

> 
> Free any framework-owned buffer before replacing it, mirroring the
> ownership check already used in snd_compr_free().
> 
> Fixes: b21c60a4edd2 ("ALSA: core: add support for compress_offload")
> Cc: stable@vger.kernel.org
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> ---
>  sound/core/compress_offload.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index 7c397b1c9231..c0ed76e1c844 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -613,6 +613,10 @@ static int snd_compr_allocate_buffer(struct snd_compr_stream *stream,
>  			return -ENOMEM;
>  	}
>  
> +	/* a prior SET_PARAMS may have left a framework-owned buffer behind */
> +	if (!stream->runtime->dma_buffer_p)
> +		kfree(stream->runtime->buffer);
> +
>  	stream->runtime->buffer = buffer;
>  	stream->runtime->buffer_size = buffer_size;
>  params:
> -- 
> 2.56.0

-- 
~Vinod