On 3/9/22 08:51, Amadeusz Sławiński wrote:
On 3/9/2022 3:22 PM, Raghu Bankapur wrote:
From: Raghu Bankapur quic_rbankapu@quiinc.com
32 bit variable is used for storing number of bytes copied to DSP, which can overflow when playback duration goes beyond 24 hours. Change data type for this variable to uint64_t to prevent overflow and related playback anomaly.
Signed-off-by: Raghu Bankapur quic_rbankapu@quicinc.com
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 9555f31c8425..57d24d89b2f4 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;
I don't think this patch should be merged as is. It changes UAPI header, most likely breaking existing user space tools. Can't overflow be handled somehow instead?
Although having looked around, it makes a bit of sense, as snd_compr_update_tstamp() copies tstamp->copied_total to 64 bit variables...
Perhaps raise protocol version? ( https://elixir.bootlin.com/linux/latest/source/include/uapi/sound/compress_o... )
This change sounded familiar, and sure enough we've already discussed this in 2019. I thought we did change things but unfortunately no, the problem is still there.
https://lore.kernel.org/alsa-devel/1560843846-4631-1-git-send-email-bgoswami...
we really want to change all the copied_total, pcm_frames and pcm_io_frames to u64 and add a new IOCTL.