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

[Takashi Iwai]

On Tue, 18 Jun 2019 10:02:59 +0200,
Pierre-Louis Bossart wrote:
> 
> 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?

Yes, it breaks ABI, and we can't take this as is.

If we need 64bit values, a new ioctl must be provided while keeping
the old one still working.


thanks,

Takashi

> >
> > 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;
> >
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>