Re: [PATCH 2/4] ALSA: compress: reject buffer geometry change on gapless next_track

[Vinod Koul <vkoul@kernel.org>]

On 07-10-26, 16:25, Peter Ujfalusi wrote:
> snd_compr_set_params() is invoked from the gapless next_track
> re-entry path, which calls snd_compr_allocate_buffer() again and lets
> buffer_size change while total_bytes_available and
> total_bytes_transferred still hold values computed for the previous
> buffer. snd_compr_calc_avail() then derives an avail/count value from
> those stale counters against the new buffer_size, and
> snd_compr_write_data() copy_from_user()s that many bytes into the
> new, potentially smaller buffer, overflowing it with user-controlled
> length and content.
> 
> Reject a next_track SET_PARAMS that changes the buffer geometry
> instead of reallocating, since total_bytes_available/transferred are
> only valid for the buffer sized during the initial SET_PARAMS.
> 
> Fixes: 9727b490e543 ("ALSA: compress: add support for gapless playback")
> Cc: stable@vger.kernel.org
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> ---
>  sound/core/compress_offload.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index c0ed76e1c844..e1cc34ba2c2c 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -673,9 +673,20 @@ snd_compr_set_params(struct snd_compr_stream *stream, unsigned long arg)
>  		if (retval)
>  			return retval;
>  
> -		retval = snd_compr_allocate_buffer(stream, params);
> -		if (retval)
> -			return -ENOMEM;
> +		if (stream->next_track) {
> +			/*
> +			 * total_bytes_available/transferred still refer to the
> +			 * buffer allocated for the current track; the geometry
> +			 * must not change underneath them.
> +			 */
> +			if (params->buffer.fragment_size != stream->runtime->fragment_size ||
> +			    params->buffer.fragments != stream->runtime->fragments)
> +				return -EINVAL;
> +		} else {
> +			retval = snd_compr_allocate_buffer(stream, params);
> +			if (retval)
> +				return -ENOMEM;
> +		}

better, i would go one step further and invoke
snd_compr_allocate_buffer() only once!
I dont think DSP expects buffers will be changed

-- 
~Vinod