[FFmpeg-devel] [PATCH] aac_fixed: fix overflow in sbr_sum_square_c

[Andreas Cadhalpun]

On 16.11.2015 15:39, Nedeljko Babic wrote:
>>> On 11.11.2015 13:46, Michael Niedermayer wrote:
>> Comments fro AAC and SBR experts very welcome!
> 
> This code was developed a while ago, but based on informations that I have
> this part of code was analysed regarding possibility of overflow and conclusion
> was that there is no valid way for causing overflow here.

I would be very interested in details about this analysis of yours.
My investigation of this code leads me to believe that actually the potential
input range for sbr_sum_square is several orders of magnitude larger than what
fits into an int32_t, but since that type is used, lots of overflows are happening,
most during the imdct calculation.

> And regarding valid range, if I remember correctly it should be 29, not 32.

Well, the input range should be 29-bits, because otherwise this function can
overflow.

So if you say that larger values are invalid, I suggest to assert that they
don't happen. See attached patch.

To prevent triggering these asserts, one can force the input to be small enough.
Doing that early enough also avoids overflows along the way.
I'll send a separate patch for that.

Best regards,
Andreas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-sbrdsp_fixed-assert-that-input-values-for-sbr_sum_sq.patch
Type: text/x-diff
Size: 1296 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151119/f577d97b/attachment.patch>