[FFmpeg-devel] [PATCH 4/4] softfloat: return one when the argument of av_sqrt_sf is negative

[Michael Niedermayer]

On Sun, Nov 08, 2015 at 05:12:37PM +0100, Andreas Cadhalpun wrote:
> On 08.11.2015 01:02, Michael Niedermayer wrote:
> > On Sun, Nov 08, 2015 at 12:09:02AM +0100, Andreas Cadhalpun wrote:
> >> Mathematically this is bogus, but it is much better than the previous
> >> behaviour: returning a random value from an out of bounds read.
> >>
> >> Returning zero will just lead to division by zero problems.
> >>
> >> This fixes av_assert2 failures in the aac_fixed decoder.
> >>
> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >> ---
> >>  libavutil/softfloat.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/libavutil/softfloat.h b/libavutil/softfloat.h
> >> index fefde8c..0a2074a 100644
> >> --- a/libavutil/softfloat.h
> >> +++ b/libavutil/softfloat.h
> >> @@ -170,6 +170,8 @@ static av_always_inline SoftFloat av_sqrt_sf(SoftFloat val)
> >>  
> >>      if (val.mant == 0)
> >>          val.exp = 0;
> >> +    else if (val.mant < 0)
> >> +        val = FLOAT_1;
> > 
> > this is IMHO not ok
> > sqrt(-1) has 3 sane possible outcomes
> > 1. the complex number i
> > 2. NaN
> > 3. abort()
> 
> OK, attached is a patch adding an assert instead.
> 
> > like division by 0 sqare roots of negative numbers in a context of
> > real values are a bug, there is likely something wrong in the code
> > that is calling sqrt with a negative number and simply returning 1
> > silently would make it hard to find and debug such bugs
> 
> Returning a random number also makes it hard to debug such bugs...
> 
> Anyway, this bug happens in sbr_gain_calc, if sbr->e_curr[e][m]
> is smaller than -1. The reason for that is sbr_sum_square_c returning
> a negative value, because accu overflows.
> I'll send a patch for that, but there are also lots of other
> overflows happening in the aac_fixed decoder.
> 
> Best regards,
> Andreas
> 

>  softfloat.h |    2 ++
>  1 file changed, 2 insertions(+)
> 743e46bfbb223cee2d34e8db896758d59dcc308a  0001-softfloat-assert-when-the-argument-of-av_sqrt_sf-is-.patch
> From 2eb7607e3ead91c2cb35076c94746405183d0320 Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Sun, 8 Nov 2015 15:15:24 +0100
> Subject: [PATCH] softfloat: assert when the argument of av_sqrt_sf is negative
> 
> The correct result can't be expressed in SoftFloat.
> Currently it returns a random value from an out of bounds read.
> 
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> ---
>  libavutil/softfloat.h | 2 ++
>  1 file changed, 2 insertions(+)

LGTM

thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151108/9ddf0765/attachment.sig>