On Sat, May 27, 2017 at 12:24:16PM +0200, wm4 wrote: > On Sat, 27 May 2017 03:56:42 +0200 > Michael Niedermayer <mich...@niedermayer.cc> wrote: > > > On Fri, May 26, 2017 at 07:06:44PM -0400, Ronald S. Bultje wrote: > > > Hi, > > > > > > On Fri, May 26, 2017 at 6:34 PM, Michael Niedermayer > > > <mich...@niedermayer.cc > > > > wrote: > > > > > > > On Fri, May 26, 2017 at 06:07:35PM -0400, Ronald S. Bultje wrote: > > > > > Hi, > > > > > > > > > > On Fri, May 26, 2017 at 5:50 PM, Michael Niedermayer > > > > <mich...@niedermayer.cc > > > > > > wrote: > > > > > > > > > > > On Fri, May 26, 2017 at 11:18:12PM +0200, Hendrik Leppkes wrote: > > > > > > > On Fri, May 26, 2017 at 11:11 PM, Michael Niedermayer > > > > > > > <mich...@niedermayer.cc> wrote: > > > > > > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov > > > > wrote: > > > > > > > >> On 26 May 2017 at 12:21, wm4 <nfx...@googlemail.com> wrote: > > > > > > > >> > > > > > > > >> > On Thu, 25 May 2017 16:10:49 +0200 > > > > > > > >> > Michael Niedermayer <mich...@niedermayer.cc> wrote: > > > > > > > >> > > > > > > > > >> > > Fixes: 1735/clusterfuzz-testcase-minimized-5350472347025408 > > > > > > > >> > > > > > > > > > >> > > Found-by: continuous fuzzing process > > > > > > https://github.com/google/oss- > > > > > > > >> > fuzz/tree/master/projects/ffmpeg > > > > > > > >> > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > > > > > > > >> > > --- > > > > > > > >> > > libavcodec/fft_template.c | 50 > > > > > > > >> > > +++++++++++++++++++++++------- > > > > > > > >> > ----------------- > > > > > > > >> > > 1 file changed, 25 insertions(+), 25 deletions(-) > > > > > > > >> > > > > > > > > > >> > > diff --git a/libavcodec/fft_template.c > > > > b/libavcodec/fft_template.c > > > > > > > >> > > index 480557f49f..e3a37e5d69 100644 > > > > > > > >> > > --- a/libavcodec/fft_template.c > > > > > > > >> > > +++ b/libavcodec/fft_template.c > > > > > > > >> > > @@ -249,7 +249,7 @@ static void fft_calc_c(FFTContext *s, > > > > > > FFTComplex *z) > > > > > > > >> > { > > > > > > > >> > > > > > > > > > >> > > int nbits, i, n, num_transforms, offset, step; > > > > > > > >> > > int n4, n2, n34; > > > > > > > >> > > - FFTSample tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, > > > > > > > >> > > tmp8; > > > > > > > >> > > + SUINT tmp1, tmp2, tmp3, tmp4, tmp5, tmp6, tmp7, tmp8; > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > I want this SUINT thing gone, not have more of it. > > > > > > > >> > _______________________________________________ > > > > > > > >> > ffmpeg-devel mailing list > > > > > > > >> > ffmpeg-devel@ffmpeg.org > > > > > > > >> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > >> > > > > > > > > >> > > > > > > > >> I agree, especially here. > > > > > > > > > > > > > > > >> Overflows should be left to happen in transforms if the input > > > > > > > >> is > > > > > > corrupt. > > > > > > > > > > > > > > > > signed int overflow is not allowed in C, if that is what you > > > > > > > > meant. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Its "undefined", which means what the result will be is not > > > > > > > defined - > > > > > > > which in such a DSP function is irrelevant, if all it causes is > > > > > > > corrupt output ... from corrupt input. > > > > > > > > > > > > no, this is not correct. > > > > > > undefined behavior does not mean the effect will be limited to > > > > > > the output. > > > > > > It could cause the process to hard lockup, trigger an exception or > > > > > > set a flag causing errors in later unrelated code. > > > > > > > > > > > > > > > > > > > We've discussed this before, if you believe this to be exploitable, > > > > > why > > > > > allow it in our repository at all? I know of no other project that > > > > > even > > > > > remotely allows such ridiculous things. Please limit exploitable code > > > > > to > > > > > your personal tree, ffmpeg is not a rootkit. > > > > > > > > please calm down, you make all kinds of statments and implications in > > > > the text above which are not true. > > > > > > > > This specific code in git triggers undefined behavior, the patch fixes > > > > this undefined behavior. > > > > If that is exploitable (which i neither claim it is nor that it isnt) > > > > its a issue that exists before the patch but not afterwards. > > > > > > > > > > > *unsigned* removes the exploit. SUINT keeps it, and is therefore part of a > > > rootkit. > > > > SUINT is defined to unsigned, if unsigned removes the issue > > so does SUINT. > > Then why is it called SUINT and not UINT.
Signed value in Unsigned INTeger type This concept is needed for fixing undefined operations efficiently. The type can always be replaced by unsigned and thats what is being done now. But it makes the code hard to understand and maintain because these values are not positive integers but signed integers. Which for C standard compliance need to be stored in a unsigned type. Both SUINT and unsigned should produce identical binaries but unsigned is semantically wrong. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Democracy is the form of government in which you can choose your dictator
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
