[FFmpeg-devel] [PATCH 3/3] avcodec/aacpsdsp_template: Fixes integer overflow in ps_add_squares_c()

wm4 nfxjfg at googlemail.com
Mon Jul 10 15:45:32 EEST 2017


On Mon, 10 Jul 2017 09:29:31 -0300
James Almer <jamrial at gmail.com> wrote:

> On 7/10/2017 5:37 AM, wm4 wrote:
> > On Sun, 9 Jul 2017 22:03:21 +0200
> > Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
> >   
> >> On 09.07.2017, at 16:08, "Ronald S. Bultje" <rsbultje at gmail.com> wrote:  
> >>> Hi,
> >>>
> >>> On Sun, Jul 9, 2017 at 4:39 AM, Reimar Döffinger <Reimar.Doeffinger at gmx.de>
> >>> wrote:
> >>>     
> >>>> On 09.07.2017, at 02:52, "Ronald S. Bultje" <rsbultje at gmail.com> wrote:    
> >>>>> On Sat, Jul 8, 2017 at 5:17 PM, Michael Niedermayer    
> >>>> <michael at niedermayer.cc>    
> >>>>> wrote:
> >>>>>     
> >>>>>>
> >>>>>> Does anyone object to this patch ?
> >>>>>> Or does anyone have a better idea on how to fix this ?
> >>>>>> if not id like to apply it    
> >>>>>
> >>>>>
> >>>>> I think Rostislav's point is: why fix it, if it can only happen with
> >>>>> corrupt input? The before and after situation is identical: garbage in,
> >>>>> garbage out. If the compiler does funny things that makes the garbage
> >>>>> slightly differently bad, is that really so devilishly bad? It's still
> >>>>> garbage. Is anything improved by this?    
> >>>>
> >>>> The way C works, you MUST assume any undefined behaviour can at any point
> >>>> [..] become exploitable.[..] If you don't like that, C is the wrong
> >>>> language to use.    
> >>>
> >>>
> >>> I think I've read "the boy who cried wolf" a few too many times to my kids,
> >>> but the form of this discussion is currently too polarizing/political for
> >>> my taste.    
> >>
> >> That is my reading of the C standard, is that political or even just controversial?
> >> I mean of course you can ignore standards (see MPEG-4 ASP, and in some ways that was actually fairly reasonable thing to do at the time), and I don't fix every undefined behaviour case in my code when I can't think of any reasonable solution.
> >> So there is a cost-benefit discussion in principle.
> >> I believe the cost of not fixing undefined behaviour, just by virtue of going outside what the standard guarantees should be considered fairly high.
> >> That is an opinion, but is there any disagreement that undefined behaviour is at least an issue of some degree?
> >> If we can agree on that, then the question would only be how much effort/code ugliness is reasonable.
> >> There is also the point (which I hope I mentioned in the parts cut out) that just making sure that these cases are not already exploitable right now with the current compiler can in many cases be quite a pain (and does not have tool support), so I think fixing it would often be the lowest-effort method.  
> > 
> > The controversial thing is actually the SUINT nonsense. A type is
> > either signed or unsigned, but not both incompletely intransparent
> > ways. Michael keeps adding them even though many are against it. 
> > 
> > "SUINTFLOAT" just tops this. Is it a signed int? Unsigned int? A float?
> > Unsigned float???  
> 
> SUINTFLOAT is float or an integer (SUINT) depending on how you include
> the template file.
> 
> Blame the template crap for all the int variants of decoders (aac, ac3,
> etc).

Yeah, I understand how it came to be, but maybe it's not a good idea to
combine the various hacks into more combinations of hacks.


More information about the ffmpeg-devel mailing list