[alsa-devel] [PATCH] ALSA: compress: avoid integer overflow for long duration offload playback

[Pierre-Louis Bossart]

On 6/18/19 9:44 AM, bgoswami at codeaurora.org wrote:
> From: Banajit Goswami <bgoswami at codeaurora.org>
> 
> Currently a 32 bit variable is used for storing number of bytes
> copied to DSP, which can overflow when playback continues for a long
> duration.
> Change data type for this variable to __u64 to prevent overflow.

This clearly looks like a bug, the number of bytes transferred is stored 
as u64 in the runtime. I have no memories of this being intentional.

That said, it seems odd to me that you have an overflow on the number of 
bytes but not on the number of PCM frames. Shouldn't both the pcm_frames 
and pcm_io_frames fields also be changed to u64 while we are at it?

And the second issue is that this may impact apps. This is a ABI change, 
isn't it?

> 
> Signed-off-by: Dhananjay Kumar <dhakumar at codeaurora.org>
> Signed-off-by: Banajit Goswami <bgoswami at codeaurora.org>
> ---
>   include/uapi/sound/compress_offload.h | 2 +-
>   sound/core/compress_offload.c         | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/sound/compress_offload.h b/include/uapi/sound/compress_offload.h
> index 56d9567..db5edf3 100644
> --- a/include/uapi/sound/compress_offload.h
> +++ b/include/uapi/sound/compress_offload.h
> @@ -67,7 +67,7 @@ struct snd_compr_params {
>    */
>   struct snd_compr_tstamp {
>   	__u32 byte_offset;
> -	__u32 copied_total;
> +	__u64 copied_total;
>   	__u32 pcm_frames;
>   	__u32 pcm_io_frames;
>   	__u32 sampling_rate;
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index a1a6fd7..2d0a709 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -184,7 +184,7 @@ static int snd_compr_update_tstamp(struct snd_compr_stream *stream,
>   	if (!stream->ops->pointer)
>   		return -ENOTSUPP;
>   	stream->ops->pointer(stream, tstamp);
> -	pr_debug("dsp consumed till %d total %d bytes\n",
> +	pr_debug("dsp consumed till %d total %llu bytes\n",
>   		tstamp->byte_offset, tstamp->copied_total);
>   	if (stream->direction == SND_COMPRESS_PLAYBACK)
>   		stream->runtime->total_bytes_transferred = tstamp->copied_total;
>