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

["Péter Ujfalusi" <peter.ujfalusi@linux.intel.com>]

On 07/10/2026 17:10, Vinod Koul wrote:
> On 07-10-26, 16:04, Takashi Iwai wrote:
>> On Wed, 07 Oct 2026 15:25:06 +0200,
>> 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.
>>>
>>> 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);
>>> +
>>
>> I'd rather put to the else block above (or even better, if
>> (stream->runtime->dma_buffer_p) block above that point).
>>
>> The code is specific to that condition, after all.
> 
> I would block calling snd_compr_allocate_buffer() for subsequent
> set_params, it should not be allowed.

OK, let me see how it should be done. I'm not sure of changing other
params are permitted either, but imagine:
the application calls set_params first then after some deliberation and
before starting it reconsiders and wants to have bigger/smaller buffer.
I know, they rarely do, but if such application assumes that the second
buffer setup is valid, while we kept the initial one, things might break?

-- 
Péter