[FFmpeg-devel,2/2] avcodec/sbrdsp_fixed: Fix integer overflow

Fixes: signed integer overflow: 2147483598 + 64 cannot be represented in type 'int'
Fixes: 4337/clusterfuzz-testcase-minimized-6192658616680448

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/sbrdsp_fixed.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

2017-11-22 21:00 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>:

> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c
> index a0ef6859f1..0db932a105 100644
> --- a/libavcodec/sbrdsp_fixed.c
> +++ b/libavcodec/sbrdsp_fixed.c
> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t accu)
>
>          round = 1U << (nz-1);
>          mant = (int)((accu + round) >> nz);
> -        mant = (mant + 0x40)>>7;
> +        mant = (mant + 0x40ll)>>7;

LL?

Carl Eugen

On Wed, Nov 22, 2017 at 11:38 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 2017-11-22 21:00 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>:
>
>> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c
>> index a0ef6859f1..0db932a105 100644
>> --- a/libavcodec/sbrdsp_fixed.c
>> +++ b/libavcodec/sbrdsp_fixed.c
>> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t accu)
>>
>>          round = 1U << (nz-1);
>>          mant = (int)((accu + round) >> nz);
>> -        mant = (mant + 0x40)>>7;
>> +        mant = (mant + 0x40ll)>>7;
>
> LL?
>

More correctly, shouldnt this use one of those fancy integer constant
macros, like INT64_C(0x40)? (I don't actually know if those are
supposed to work with hex constants, but the fact that they exist
seems to indicate that LL is not entirely portable)

- Hendrik

On Wed, Nov 22, 2017 at 11:59:30PM +0100, Hendrik Leppkes wrote:
> On Wed, Nov 22, 2017 at 11:38 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> > 2017-11-22 21:00 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>:
> >
> >> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c
> >> index a0ef6859f1..0db932a105 100644
> >> --- a/libavcodec/sbrdsp_fixed.c
> >> +++ b/libavcodec/sbrdsp_fixed.c
> >> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t accu)
> >>
> >>          round = 1U << (nz-1);
> >>          mant = (int)((accu + round) >> nz);
> >> -        mant = (mant + 0x40)>>7;
> >> +        mant = (mant + 0x40ll)>>7;
> >
> > LL?

I can change it to LL if preferred, i guess ill do that, LL seems more
common in our source


> >
> 
> More correctly, shouldnt this use one of those fancy integer constant
> macros, like INT64_C(0x40)? (I don't actually know if those are
> supposed to work with hex constants, but the fact that they exist
> seems to indicate that LL is not entirely portable)

we use both LL and ll already, so there should be no issue that we
arent already affected by since "forever"

[...]

On 11/22/2017 7:59 PM, Hendrik Leppkes wrote:
> On Wed, Nov 22, 2017 at 11:38 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>> 2017-11-22 21:00 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>:
>>
>>> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c
>>> index a0ef6859f1..0db932a105 100644
>>> --- a/libavcodec/sbrdsp_fixed.c
>>> +++ b/libavcodec/sbrdsp_fixed.c
>>> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t accu)
>>>
>>>          round = 1U << (nz-1);
>>>          mant = (int)((accu + round) >> nz);
>>> -        mant = (mant + 0x40)>>7;
>>> +        mant = (mant + 0x40ll)>>7;
>>
>> LL?
>>
> 
> More correctly, shouldnt this use one of those fancy integer constant
> macros, like INT64_C(0x40)? (I don't actually know if those are
> supposed to work with hex constants, but the fact that they exist
> seems to indicate that LL is not entirely portable)
> 
> - Hendrik

LL/ULL are used plenty of times in the tree, so it's portable enough.
What i remember not working at least on old msvc was LLU.

On Thu, Nov 23, 2017 at 11:00:21PM +0100, Michael Niedermayer wrote:
> On Wed, Nov 22, 2017 at 11:59:30PM +0100, Hendrik Leppkes wrote:
> > On Wed, Nov 22, 2017 at 11:38 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> > > 2017-11-22 21:00 GMT+01:00 Michael Niedermayer <michael@niedermayer.cc>:
> > >
> > >> diff --git a/libavcodec/sbrdsp_fixed.c b/libavcodec/sbrdsp_fixed.c
> > >> index a0ef6859f1..0db932a105 100644
> > >> --- a/libavcodec/sbrdsp_fixed.c
> > >> +++ b/libavcodec/sbrdsp_fixed.c
> > >> @@ -133,7 +133,7 @@ static av_always_inline SoftFloat autocorr_calc(int64_t accu)
> > >>
> > >>          round = 1U << (nz-1);
> > >>          mant = (int)((accu + round) >> nz);
> > >> -        mant = (mant + 0x40)>>7;
> > >> +        mant = (mant + 0x40ll)>>7;
> > >
> > > LL?
> 
> I can change it to LL if preferred, i guess ill do that, LL seems more
> common in our source

and applied with LL


[...]