| Message ID | 20170525141049.23344-1-michael@niedermayer.cc |
|---|---|
| State | Superseded |
| Headers | show |
On 25 May 2017 at 15:10, Michael Niedermayer <michael@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 > <https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg%0ASigned-off-by>: > Michael Niedermayer <michael@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; > FFTComplex *tmpz; > const int fft_size = (1 << s->nbits); > int64_t accu; > @@ -260,14 +260,14 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) > { > offset = ff_fft_offsets_lut[n] << 2; > tmpz = z + offset; > > - tmp1 = tmpz[0].re + tmpz[1].re; > - tmp5 = tmpz[2].re + tmpz[3].re; > - tmp2 = tmpz[0].im + tmpz[1].im; > - tmp6 = tmpz[2].im + tmpz[3].im; > - tmp3 = tmpz[0].re - tmpz[1].re; > - tmp8 = tmpz[2].im - tmpz[3].im; > - tmp4 = tmpz[0].im - tmpz[1].im; > - tmp7 = tmpz[2].re - tmpz[3].re; > + tmp1 = tmpz[0].re + (SUINT)tmpz[1].re; > + tmp5 = tmpz[2].re + (SUINT)tmpz[3].re; > + tmp2 = tmpz[0].im + (SUINT)tmpz[1].im; > + tmp6 = tmpz[2].im + (SUINT)tmpz[3].im; > + tmp3 = tmpz[0].re - (SUINT)tmpz[1].re; > + tmp8 = tmpz[2].im - (SUINT)tmpz[3].im; > + tmp4 = tmpz[0].im - (SUINT)tmpz[1].im; > + tmp7 = tmpz[2].re - (SUINT)tmpz[3].re; > > tmpz[0].re = tmp1 + tmp5; > tmpz[2].re = tmp1 - tmp5; > @@ -288,19 +288,19 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) > { > offset = ff_fft_offsets_lut[n] << 3; > tmpz = z + offset; > > - tmp1 = tmpz[4].re + tmpz[5].re; > - tmp3 = tmpz[6].re + tmpz[7].re; > - tmp2 = tmpz[4].im + tmpz[5].im; > - tmp4 = tmpz[6].im + tmpz[7].im; > + tmp1 = tmpz[4].re + (SUINT)tmpz[5].re; > + tmp3 = tmpz[6].re + (SUINT)tmpz[7].re; > + tmp2 = tmpz[4].im + (SUINT)tmpz[5].im; > + tmp4 = tmpz[6].im + (SUINT)tmpz[7].im; > tmp5 = tmp1 + tmp3; > tmp7 = tmp1 - tmp3; > tmp6 = tmp2 + tmp4; > tmp8 = tmp2 - tmp4; > > - tmp1 = tmpz[4].re - tmpz[5].re; > - tmp2 = tmpz[4].im - tmpz[5].im; > - tmp3 = tmpz[6].re - tmpz[7].re; > - tmp4 = tmpz[6].im - tmpz[7].im; > + tmp1 = tmpz[4].re - (SUINT)tmpz[5].re; > + tmp2 = tmpz[4].im - (SUINT)tmpz[5].im; > + tmp3 = tmpz[6].re - (SUINT)tmpz[7].re; > + tmp4 = tmpz[6].im - (SUINT)tmpz[7].im; > > tmpz[4].re = tmpz[0].re - tmp5; > tmpz[0].re = tmpz[0].re + tmp5; > @@ -311,13 +311,13 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) > { > tmpz[6].im = tmpz[2].im + tmp7; > tmpz[2].im = tmpz[2].im - tmp7; > > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp1 + tmp2); > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp1 + tmp2); > tmp5 = (int32_t)((accu + 0x40000000) >> 31); > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp3 - tmp4); > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp3 - tmp4); > tmp7 = (int32_t)((accu + 0x40000000) >> 31); > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp2 - tmp1); > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp2 - tmp1); > tmp6 = (int32_t)((accu + 0x40000000) >> 31); > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp3 + tmp4); > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp3 + tmp4); > tmp8 = (int32_t)((accu + 0x40000000) >> 31); > tmp1 = tmp5 + tmp7; > tmp3 = tmp5 - tmp7; > @@ -348,10 +348,10 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) > { > offset = ff_fft_offsets_lut[n] << nbits; > tmpz = z + offset; > > - tmp5 = tmpz[ n2].re + tmpz[n34].re; > - tmp1 = tmpz[ n2].re - tmpz[n34].re; > - tmp6 = tmpz[ n2].im + tmpz[n34].im; > - tmp2 = tmpz[ n2].im - tmpz[n34].im; > + tmp5 = tmpz[ n2].re + (SUINT)tmpz[n34].re; > + tmp1 = tmpz[ n2].re - (SUINT)tmpz[n34].re; > + tmp6 = tmpz[ n2].im + (SUINT)tmpz[n34].im; > + tmp2 = tmpz[ n2].im - (SUINT)tmpz[n34].im; > > tmpz[ n2].re = tmpz[ 0].re - tmp5; > tmpz[ 0].re = tmpz[ 0].re + tmp5; > -- > 2.13.0 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > libavcodec/avfft.h:typedef float FFTSample; libavcodec/fft.h:typedef int32_t FFTSample; libavcodec/fft.h:typedef int16_t FFTSample; You're forcing something to be integer in a templated file, and also I'm unhappy with how you change typedef'd types to this SUINT thing. Please never change typedef'd types even if they're always integer and instead change the typedef to go from "typedef int <something>" to "typedef SUINT <something>". Things are typedeffed for a reason and clarity (e.g. to not mislead devs into thinking they're just integers and there's nothing special going on).
On Thu, May 25, 2017 at 03:29:59PM +0100, Rostislav Pehlivanov wrote: > On 25 May 2017 at 15:10, Michael Niedermayer <michael@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 > > <https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg%0ASigned-off-by>: > > Michael Niedermayer <michael@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; > > FFTComplex *tmpz; > > const int fft_size = (1 << s->nbits); > > int64_t accu; > > @@ -260,14 +260,14 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) > > { > > offset = ff_fft_offsets_lut[n] << 2; > > tmpz = z + offset; > > > > - tmp1 = tmpz[0].re + tmpz[1].re; > > - tmp5 = tmpz[2].re + tmpz[3].re; > > - tmp2 = tmpz[0].im + tmpz[1].im; > > - tmp6 = tmpz[2].im + tmpz[3].im; > > - tmp3 = tmpz[0].re - tmpz[1].re; > > - tmp8 = tmpz[2].im - tmpz[3].im; > > - tmp4 = tmpz[0].im - tmpz[1].im; > > - tmp7 = tmpz[2].re - tmpz[3].re; > > + tmp1 = tmpz[0].re + (SUINT)tmpz[1].re; > > + tmp5 = tmpz[2].re + (SUINT)tmpz[3].re; > > + tmp2 = tmpz[0].im + (SUINT)tmpz[1].im; > > + tmp6 = tmpz[2].im + (SUINT)tmpz[3].im; > > + tmp3 = tmpz[0].re - (SUINT)tmpz[1].re; > > + tmp8 = tmpz[2].im - (SUINT)tmpz[3].im; > > + tmp4 = tmpz[0].im - (SUINT)tmpz[1].im; > > + tmp7 = tmpz[2].re - (SUINT)tmpz[3].re; > > > > tmpz[0].re = tmp1 + tmp5; > > tmpz[2].re = tmp1 - tmp5; > > @@ -288,19 +288,19 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) > > { > > offset = ff_fft_offsets_lut[n] << 3; > > tmpz = z + offset; > > > > - tmp1 = tmpz[4].re + tmpz[5].re; > > - tmp3 = tmpz[6].re + tmpz[7].re; > > - tmp2 = tmpz[4].im + tmpz[5].im; > > - tmp4 = tmpz[6].im + tmpz[7].im; > > + tmp1 = tmpz[4].re + (SUINT)tmpz[5].re; > > + tmp3 = tmpz[6].re + (SUINT)tmpz[7].re; > > + tmp2 = tmpz[4].im + (SUINT)tmpz[5].im; > > + tmp4 = tmpz[6].im + (SUINT)tmpz[7].im; > > tmp5 = tmp1 + tmp3; > > tmp7 = tmp1 - tmp3; > > tmp6 = tmp2 + tmp4; > > tmp8 = tmp2 - tmp4; > > > > - tmp1 = tmpz[4].re - tmpz[5].re; > > - tmp2 = tmpz[4].im - tmpz[5].im; > > - tmp3 = tmpz[6].re - tmpz[7].re; > > - tmp4 = tmpz[6].im - tmpz[7].im; > > + tmp1 = tmpz[4].re - (SUINT)tmpz[5].re; > > + tmp2 = tmpz[4].im - (SUINT)tmpz[5].im; > > + tmp3 = tmpz[6].re - (SUINT)tmpz[7].re; > > + tmp4 = tmpz[6].im - (SUINT)tmpz[7].im; > > > > tmpz[4].re = tmpz[0].re - tmp5; > > tmpz[0].re = tmpz[0].re + tmp5; > > @@ -311,13 +311,13 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) > > { > > tmpz[6].im = tmpz[2].im + tmp7; > > tmpz[2].im = tmpz[2].im - tmp7; > > > > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp1 + tmp2); > > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp1 + tmp2); > > tmp5 = (int32_t)((accu + 0x40000000) >> 31); > > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp3 - tmp4); > > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp3 - tmp4); > > tmp7 = (int32_t)((accu + 0x40000000) >> 31); > > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp2 - tmp1); > > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp2 - tmp1); > > tmp6 = (int32_t)((accu + 0x40000000) >> 31); > > - accu = (int64_t)Q31(M_SQRT1_2)*(tmp3 + tmp4); > > + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp3 + tmp4); > > tmp8 = (int32_t)((accu + 0x40000000) >> 31); > > tmp1 = tmp5 + tmp7; > > tmp3 = tmp5 - tmp7; > > @@ -348,10 +348,10 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) > > { > > offset = ff_fft_offsets_lut[n] << nbits; > > tmpz = z + offset; > > > > - tmp5 = tmpz[ n2].re + tmpz[n34].re; > > - tmp1 = tmpz[ n2].re - tmpz[n34].re; > > - tmp6 = tmpz[ n2].im + tmpz[n34].im; > > - tmp2 = tmpz[ n2].im - tmpz[n34].im; > > + tmp5 = tmpz[ n2].re + (SUINT)tmpz[n34].re; > > + tmp1 = tmpz[ n2].re - (SUINT)tmpz[n34].re; > > + tmp6 = tmpz[ n2].im + (SUINT)tmpz[n34].im; > > + tmp2 = tmpz[ n2].im - (SUINT)tmpz[n34].im; > > > > tmpz[ n2].re = tmpz[ 0].re - tmp5; > > tmpz[ 0].re = tmpz[ 0].re + tmp5; > > -- > > 2.13.0 > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > libavcodec/avfft.h:typedef float FFTSample; > libavcodec/fft.h:typedef int32_t FFTSample; > libavcodec/fft.h:typedef int16_t FFTSample; > > You're forcing something to be integer in a templated file, and also I'm > unhappy with how you change typedef'd types to this SUINT thing. > Please never change typedef'd types even if they're always integer and > instead change the typedef to go from "typedef int <something>" to "typedef > SUINT <something>". Things are typedeffed for a reason and clarity (e.g. to > not mislead devs into thinking they're just integers and there's nothing > special going on). The changed code is under #if FFT_FIXED_32 so the changed type is always int32 and FFTSample is actually misleading as it suggests it can be something else, especially in a template. Not that i really have an oppinion on this, just pointing it out. Iam not sure i understand what exactly you suggest but I can introduce a SUFFTSample typdef if thats preferred? or do you prefer / meant something else ? [...]
On Thu, 25 May 2017 16:10:49 +0200 Michael Niedermayer <michael@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 <michael@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.
On 26 May 2017 at 12:21, wm4 <nfxjfg@googlemail.com> wrote: > On Thu, 25 May 2017 16:10:49 +0200 > Michael Niedermayer <michael@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 <michael@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. Codecs are designed such that transforms won't overflow unless corrupt data is fed. We allow for that to happen already (in the VP9 DCTs), so FFTs shouldn't be excluded.
On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov wrote: > On 26 May 2017 at 12:21, wm4 <nfxjfg@googlemail.com> wrote: > > > On Thu, 25 May 2017 16:10:49 +0200 > > Michael Niedermayer <michael@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 <michael@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. > Codecs are designed such that transforms won't overflow unless corrupt data > is fed. We allow for that to happen already (in the VP9 DCTs), so FFTs > shouldn't be excluded. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Fri, May 26, 2017 at 11:11 PM, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov wrote: >> On 26 May 2017 at 12:21, wm4 <nfxjfg@googlemail.com> wrote: >> >> > On Thu, 25 May 2017 16:10:49 +0200 >> > Michael Niedermayer <michael@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 <michael@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. Its not like SUINT actually fixes them, it just silences them in debug builds. - Hendrik
On Fri, May 26, 2017 at 11:18:12PM +0200, Hendrik Leppkes wrote: > On Fri, May 26, 2017 at 11:11 PM, Michael Niedermayer > <michael@niedermayer.cc> wrote: > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov wrote: > >> On 26 May 2017 at 12:21, wm4 <nfxjfg@googlemail.com> wrote: > >> > >> > On Thu, 25 May 2017 16:10:49 +0200 > >> > Michael Niedermayer <michael@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 <michael@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. A C compiler can assume that undefined behavior does not occur so it can completely ignore any possibility that implies undefined behavior. As referece the text from iso C: undefined behavior behavior, upon use of a nonportable or erroneous program construct or of erroneous data, for which this International Standard imposes no requirements NOTE Possible undefined behavior ranges from ignoring the situation completely with unpredictable results, to behaving during translation or program execution in a documented manner characteristic of the environment (with or without the issuance of a diagnostic message), to terminating a translation or execution (with the issuance of a diagnostic message). EXAMPLE An example of undefined behavior is the behavior on integer overflow. [...]
Hi, On Fri, May 26, 2017 at 5:50 PM, Michael Niedermayer <michael@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 > > <michael@niedermayer.cc> wrote: > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov wrote: > > >> On 26 May 2017 at 12:21, wm4 <nfxjfg@googlemail.com> wrote: > > >> > > >> > On Thu, 25 May 2017 16:10:49 +0200 > > >> > Michael Niedermayer <michael@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 <michael@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. Ronald
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 <michael@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 > > > <michael@niedermayer.cc> wrote: > > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov wrote: > > > >> On 26 May 2017 at 12:21, wm4 <nfxjfg@googlemail.com> wrote: > > > >> > > > >> > On Thu, 25 May 2017 16:10:49 +0200 > > > >> > Michael Niedermayer <michael@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 <michael@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. [...]
Hi, On Fri, May 26, 2017 at 6:34 PM, Michael Niedermayer <michael@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 > <michael@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 > > > > <michael@niedermayer.cc> wrote: > > > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov > wrote: > > > > >> On 26 May 2017 at 12:21, wm4 <nfxjfg@googlemail.com> wrote: > > > > >> > > > > >> > On Thu, 25 May 2017 16:10:49 +0200 > > > > >> > Michael Niedermayer <michael@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 <michael@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. Or there is no exploit, in which case SUINT is not necessary. ?? Ronald
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 <michael@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 > > <michael@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 > > > > > <michael@niedermayer.cc> wrote: > > > > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov > > wrote: > > > > > >> On 26 May 2017 at 12:21, wm4 <nfxjfg@googlemail.com> wrote: > > > > > >> > > > > > >> > On Thu, 25 May 2017 16:10:49 +0200 > > > > > >> > Michael Niedermayer <michael@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 <michael@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. If an attacker can convince his victim to define or redefine a symbol during build then SUINT is neither needed nor anything an attacker would choose. redefining SUINT makes sense to developers though to test for arithmetic overflow. It also keeps a clear seperation between unsigned values in unsigned types and signed values in unsigned types aka "SUINT" > > Or there is no exploit, in which case SUINT is not necessary. > > ?? > > Ronald > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Sat, 27 May 2017 03:56:42 +0200 Michael Niedermayer <michael@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 <michael@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 > > > <michael@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 > > > > > > <michael@niedermayer.cc> wrote: > > > > > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov > > > wrote: > > > > > > >> On 26 May 2017 at 12:21, wm4 <nfxjfg@googlemail.com> wrote: > > > > > > >> > > > > > > >> > On Thu, 25 May 2017 16:10:49 +0200 > > > > > > >> > Michael Niedermayer <michael@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 <michael@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.
On Sat, May 27, 2017 at 12:24:16PM +0200, wm4 wrote: > On Sat, 27 May 2017 03:56:42 +0200 > Michael Niedermayer <michael@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 <michael@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 > > > > <michael@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 > > > > > > > <michael@niedermayer.cc> wrote: > > > > > > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov > > > > wrote: > > > > > > > >> On 26 May 2017 at 12:21, wm4 <nfxjfg@googlemail.com> wrote: > > > > > > > >> > > > > > > > >> > On Thu, 25 May 2017 16:10:49 +0200 > > > > > > > >> > Michael Niedermayer <michael@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 <michael@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. [...]
Hi, On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer <michael@niedermayer.cc> wrote: > Signed value in > Unsigned > INTeger type [..] > Both SUINT and unsigned should produce identical binaries This seems to go against the rule that code should be as simple as possible. Unsigned is simpler than SUINT if the outcome is the same. Ronald
On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote: > Hi, > > On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > Signed value in > > Unsigned > > INTeger type > > [..] > > Both SUINT and unsigned should produce identical binaries > > This seems to go against the rule that code should be as simple as possible. > > Unsigned is simpler than SUINT if the outcome is the same. You can simply add the part of my mail here as awnser that you snipped away: "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." A type that avoids the undefinedness of signed but is semantically signed is correct, unsigned is not. If understandable code and maintainable code has no value to you, you would favour using single letter variables exclusivly and would never use typedef. But you do not do that. I fail to understand why you insist on using unsigned in place of a more specific type, it is not the correct nor clean thing to do. [...]
On 6/9/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote: >> Hi, >> >> On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer >> <michael@niedermayer.cc> >> wrote: >> >> > Signed value in >> > Unsigned >> > INTeger type >> >> [..] >> > Both SUINT and unsigned should produce identical binaries >> >> This seems to go against the rule that code should be as simple as >> possible. >> >> Unsigned is simpler than SUINT if the outcome is the same. > > You can simply add the part of my mail here as awnser that you snipped > away: > > "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." > > A type that avoids the undefinedness of signed but is semantically > signed is correct, unsigned is not. > > If understandable code and maintainable code has no value to you, > you would favour using single letter variables exclusivly and would > never use typedef. > But you do not do that. > > I fail to understand why you insist on using unsigned in place of a > more specific type, it is not the correct nor clean thing to do. If I understand correctly, the root of the problem is the undefined behavior of the compiler and the C standard. Is there any chance that FFmpeg project could initiate a change in the next C standard, so this thing would be defined? (and I guess, also define a few other similar things, like signed right shift, etc...) It is a matter of fact, that a lot of compiler-defined-things have been implemented in certain ways on most compilers and that there are plenty of programs depend on these specific implementations and break when a new compiler does things differently (clang had nice article about it). About the typedef, is SUINT a standard defined type for workarounding this specific problem? I do suspect that some of our fellow developers simply find its name ugly. Maybe they would be more welcoming if "suint32_t" like typedef is used?
On Fri, 9 Jun 2017 11:34:26 +0300 Ivan Kalvachev <ikalvachev@gmail.com> wrote: > > If I understand correctly, the root of the problem is the undefined behavior > of the compiler and the C standard. > Is there any chance that FFmpeg project could initiate a change in the > next C standard, so this thing would be defined? > (and I guess, also define a few other similar things, like signed > right shift, etc...) > > It is a matter of fact, that a lot of compiler-defined-things have > been implemented in > certain ways on most compilers and that there are plenty of programs > depend on these > specific implementations and break when a new compiler does things differently > (clang had nice article about it). > > > About the typedef, is SUINT a standard defined type for workarounding > this specific problem? > I do suspect that some of our fellow developers simply find its name ugly. > > Maybe they would be more welcoming if > "suint32_t" like typedef is used? This thing is brain dead - not even the post year 2000 C committee would accept it.
On Fri, 9 Jun 2017 00:10:49 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote: > On Sat, May 27, 2017 at 12:24:16PM +0200, wm4 wrote: > > On Sat, 27 May 2017 03:56:42 +0200 > > Michael Niedermayer <michael@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 <michael@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 > > > > > <michael@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 > > > > > > > > <michael@niedermayer.cc> wrote: > > > > > > > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav Pehlivanov > > > > > wrote: > > > > > > > > >> On 26 May 2017 at 12:21, wm4 <nfxjfg@googlemail.com> wrote: > > > > > > > > >> > > > > > > > > >> > On Thu, 25 May 2017 16:10:49 +0200 > > > > > > > > >> > Michael Niedermayer <michael@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 <michael@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 that so? Then please add a FATE check that guarantees this. > is semantically wrong. If it's wrong then it shouldn't be used.
On 6/9/17, wm4 <nfxjfg@googlemail.com> wrote: > On Fri, 9 Jun 2017 11:34:26 +0300 > Ivan Kalvachev <ikalvachev@gmail.com> wrote: > >> >> If I understand correctly, the root of the problem is the undefined >> behavior >> of the compiler and the C standard. >> Is there any chance that FFmpeg project could initiate a change in the >> next C standard, so this thing would be defined? >> (and I guess, also define a few other similar things, like signed >> right shift, etc...) >> >> It is a matter of fact, that a lot of compiler-defined-things have >> been implemented in >> certain ways on most compilers and that there are plenty of programs >> depend on these >> specific implementations and break when a new compiler does things >> differently >> (clang had nice article about it). >> >> >> About the typedef, is SUINT a standard defined type for workarounding >> this specific problem? >> I do suspect that some of our fellow developers simply find its name ugly. >> >> Maybe they would be more welcoming if >> "suint32_t" like typedef is used? > > This thing is brain dead - not even the post year 2000 C committee > would accept it. Also please stop spreading hacks to overcome various "timeouts" from those fuzzers.
On 6/9/17, wm4 <nfxjfg@googlemail.com> wrote: > On Fri, 9 Jun 2017 00:10:49 +0200 > Michael Niedermayer <michael@niedermayer.cc> wrote: > >> On Sat, May 27, 2017 at 12:24:16PM +0200, wm4 wrote: >> > On Sat, 27 May 2017 03:56:42 +0200 >> > Michael Niedermayer <michael@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 >> > > > <michael@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 >> > > > > <michael@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 >> > > > > > > > <michael@niedermayer.cc> wrote: >> > > > > > > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav >> > > > > > > > > Pehlivanov >> > > > > wrote: >> > > > > > > > >> On 26 May 2017 at 12:21, wm4 <nfxjfg@googlemail.com> >> > > > > > > > >> wrote: >> > > > > > > > >> >> > > > > > > > >> > On Thu, 25 May 2017 16:10:49 +0200 >> > > > > > > > >> > Michael Niedermayer <michael@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 >> > > > > > > > >> > > <michael@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 that so? Then please add a FATE check that guarantees this. > >> is semantically wrong. > > If it's wrong then it shouldn't be used. I think you should read (again) these articles: http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html
Hi, On Thu, Jun 8, 2017 at 8:57 PM, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote: > > Hi, > > > > On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer > <michael@niedermayer.cc> > > wrote: > > > > > Signed value in > > > Unsigned > > > INTeger type > > > > [..] > > > Both SUINT and unsigned should produce identical binaries > > > > This seems to go against the rule that code should be as simple as > possible. > > > > Unsigned is simpler than SUINT if the outcome is the same. > > You can simply add the part of my mail here as awnser that you snipped > away: > > "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." > > A type that avoids the undefinedness of signed but is semantically > signed is correct, unsigned is not. > > If understandable code and maintainable code has no value to you, > you would favour using single letter variables exclusivly and would > never use typedef. > But you do not do that. > > I fail to understand why you insist on using unsigned in place of a > more specific type, it is not the correct nor clean thing to do. It's not just me, it appears to be most of us. Can't you just step back at some point and be like "ok, I'll let the majority have their way"? Ronald
On Sat, 10 Jun 2017 02:50:11 +0300 Ivan Kalvachev <ikalvachev@gmail.com> wrote: > On 6/9/17, wm4 <nfxjfg@googlemail.com> wrote: > > On Fri, 9 Jun 2017 00:10:49 +0200 > > Michael Niedermayer <michael@niedermayer.cc> wrote: > > > >> On Sat, May 27, 2017 at 12:24:16PM +0200, wm4 wrote: > >> > On Sat, 27 May 2017 03:56:42 +0200 > >> > Michael Niedermayer <michael@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 > >> > > > <michael@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 > >> > > > > <michael@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 > >> > > > > > > > <michael@niedermayer.cc> wrote: > >> > > > > > > > > On Fri, May 26, 2017 at 03:20:14PM +0100, Rostislav > >> > > > > > > > > Pehlivanov > >> > > > > wrote: > >> > > > > > > > >> On 26 May 2017 at 12:21, wm4 <nfxjfg@googlemail.com> > >> > > > > > > > >> wrote: > >> > > > > > > > >> > >> > > > > > > > >> > On Thu, 25 May 2017 16:10:49 +0200 > >> > > > > > > > >> > Michael Niedermayer <michael@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 > >> > > > > > > > >> > > <michael@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 that so? Then please add a FATE check that guarantees this. > > > >> is semantically wrong. > > > > If it's wrong then it shouldn't be used. > > I think you should read (again) these articles: > http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html I know what undefined behavior is.
On 6/10/17, Ronald S. Bultje <rsbultje@gmail.com> wrote: > Hi, > > On Thu, Jun 8, 2017 at 8:57 PM, Michael Niedermayer <michael@niedermayer.cc> > wrote: > >> On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote: >> > Hi, >> > >> > On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer >> <michael@niedermayer.cc> >> > wrote: >> > >> > > Signed value in >> > > Unsigned >> > > INTeger type >> > >> > [..] >> > > Both SUINT and unsigned should produce identical binaries >> > >> > This seems to go against the rule that code should be as simple as >> possible. >> > >> > Unsigned is simpler than SUINT if the outcome is the same. >> >> You can simply add the part of my mail here as awnser that you snipped >> away: >> >> "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." >> >> A type that avoids the undefinedness of signed but is semantically >> signed is correct, unsigned is not. >> >> If understandable code and maintainable code has no value to you, >> you would favour using single letter variables exclusivly and would >> never use typedef. >> But you do not do that. >> >> I fail to understand why you insist on using unsigned in place of a >> more specific type, it is not the correct nor clean thing to do. > > > It's not just me, it appears to be most of us. Can't you just step back at > some point and be like "ok, I'll let the majority have their way"? There was actually a technical discussion undergoing, until your regular "majority" group came with strong words, opinions, and comments that clearly indicate that you don't understand the issue at hand. I'll try to explain the issue one more time. --- You all know that singed overflow is undefined. In gcc there are two options to define/control the behavior of it: -fwrapv - defines signed overflow as wrapping around, just like unsigned. -ftrapv - causes a trap exception, could lead to termination of the program. Now, as the article http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html explains in detail, undefined behavior allows certain optimizations, like loop optimizations, pointer arithmetic etc.. Using -fwrapv globally might lead to a significant slow down. On the other side, some of these optimization might lead to security exploits e.g. if they optimize-out checks in the code, that are there to prevent overflows. So there is a possible scenario, where some entity that want to secure the video playback of their browser, would like to enable a bunch of runtime checking functionality, like -fstack-protector and -fwrapv. Now, we get to the FFT. As Rostislav (atomnuker) said, it is not big issue if overflow happens in fft calculation and produces wrong result. However if -ftrapv is enabled, it may crash the whole program (!!) (instead of producing a short noise). To prevent that Michael changes the code so a signed variable is converted to unsigned for the operations that could overflow and converted back to signed for operations that need sign extension. He is using a new typedef, so the developers(!) could know that this type contains signed value, while the type for the compiler(!) is actually unsigned. Now... I would be happy if you all could think of a better way to get the same result, that doesn't involve changing types back and forth. All I can think of is: 1. Moving the functions to a special file and compiling it with -fwrapv -fno-trapv. In the fft case this might be extra hard, as the file is actually a template... 2. Asking gcc for attribute that defines the behavior, for a single function or code block. 3. Asking gcc for attribute that defines the behavior, for a variable or type. 4. Defining that behavior by the standard committee in next C standard. Maybe with new standard type. Or making "int32_t" wrap, while keeping "int" undefined. Michael solution may look ugly, make the code harder to understand, but it: 1. Works Now. 2. Does work on all compilers. 3. Doesn't make the code slower. 4. Doesn't complicate the build system. Of course, as FFmpeg developer, it is your right to initiate a vote that would prevent Michael from trying to make FFmpeg more secure. He has always complied with official decisions. However this might turn into publicity nightmare, as security community is known to overreact on certain topics.
On Fri, Jun 09, 2017 at 09:07:39PM -0400, Ronald S. Bultje wrote: > Hi, > > On Thu, Jun 8, 2017 at 8:57 PM, Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote: > > > Hi, > > > > > > On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer > > <michael@niedermayer.cc> > > > wrote: > > > > > > > Signed value in > > > > Unsigned > > > > INTeger type > > > > > > [..] > > > > Both SUINT and unsigned should produce identical binaries > > > > > > This seems to go against the rule that code should be as simple as > > possible. > > > > > > Unsigned is simpler than SUINT if the outcome is the same. > > > > You can simply add the part of my mail here as awnser that you snipped > > away: > > > > "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." > > > > A type that avoids the undefinedness of signed but is semantically > > signed is correct, unsigned is not. > > > > If understandable code and maintainable code has no value to you, > > you would favour using single letter variables exclusivly and would > > never use typedef. > > But you do not do that. > > > > I fail to understand why you insist on using unsigned in place of a > > more specific type, it is not the correct nor clean thing to do. > > > It's not just me, it appears to be most of us. Can't you just step back at > some point and be like "ok, I'll let the majority have their way"? I do not know what the majority prefers. What i see is that the people objecting are always the same 3-4 people. And very often they have no authorship or past activity in the code a patch is about. At least none i could find quickly. To me that makes these objections seem a bit out of place, more so because i just dont "get it" why people are against using a more specific type. The majority might prefer either way, i have no idea ... I very much want to choose the types and style the people activly working on some code prefer. But when i look at git shortlog of code and look at the copyright/author statments and look at MAINTAINERs and compare to the names in a thread i often find no real match. Also as i said, i belive a more specific type makes maintaince easier, thats why i want to use it in code i maintain. In code others maintain, if they see no value in it theres really alot less an argument to use it. with a specific type in this case here one can add a line and test if the FFT overflows and where it does so, if it ever produces a bad result for a real world file. With just unsigned theres no easy way to find a overflow, one has to painstakingly test this line by line by hand. If the people who intend to debug such issues see no value in a more specific type, no sense in using one. Iam fighting on this issue because i see this pushing FFmpeg into a direction where the code is harder to understand and harder to maintain and we already have many open bugs Do the people objecting to SUINT volunteer to do the extra maintaince work that may be caused by not using it ? or do they expect the existing maintainers not to use SUINT and then also do the extra work? its that 2nd case that offends me as iam one of the people who tries to maintain some of the code in question (not the fft of this patch here specifically) thanks [...]
On Sun, 11 Jun 2017 03:58:30 +0300 Ivan Kalvachev <ikalvachev@gmail.com> wrote: > Of course, as FFmpeg developer, it is your right to initiate a vote > that would prevent Michael from trying to make FFmpeg more secure. > He has always complied with official decisions. Nothing but polemic nonsense intended to scare others into having your way. If our code is so tricky that nobody can understand it or the intention behind code (like types being simultaneously signed and unsigned), it won't have a positive influence on the security of the code. If you really want to make code more secure, you should probably think about making code _simpler_, nor more complex. > However this might turn into publicity nightmare, > as security community is known to overreact > on certain topics. That too. The security community in particular seems to be full of individuals who will gladly misrepresent risks to get attentions, and companies doing the same to sell their "security" products.
On Sun, 11 Jun 2017 13:26:44 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote: > Iam fighting on this issue because i see this pushing FFmpeg into a > direction where the code is harder to understand and harder to maintain > and we already have many open bugs That's funny, because whenever I see something strange in e.g. libavformat/utils.c, I run git blame, and amazingly often you turn out as original author. Layering hack upon hack to fix specific issues. That might be because you're a very active developer, or because FFmpeg's development style forces anyone to do this "layering hacks upon hacks" to fix bugs. But still, I think that supports rather my point than yours. Also, none of these fuzz fixes close any open bugs as far as I'm aware.
On 6/11/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Fri, Jun 09, 2017 at 09:07:39PM -0400, Ronald S. Bultje wrote: >> Hi, >> >> On Thu, Jun 8, 2017 at 8:57 PM, Michael Niedermayer >> <michael@niedermayer.cc> >> wrote: >> >> > On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote: >> > > Hi, >> > > >> > > On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer >> > <michael@niedermayer.cc> >> > > wrote: >> > > >> > > > Signed value in >> > > > Unsigned >> > > > INTeger type >> > > >> > > [..] >> > > > Both SUINT and unsigned should produce identical binaries >> > > >> > > This seems to go against the rule that code should be as simple as >> > possible. >> > > >> > > Unsigned is simpler than SUINT if the outcome is the same. >> > >> > You can simply add the part of my mail here as awnser that you snipped >> > away: >> > >> > "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." >> > >> > A type that avoids the undefinedness of signed but is semantically >> > signed is correct, unsigned is not. >> > >> > If understandable code and maintainable code has no value to you, >> > you would favour using single letter variables exclusivly and would >> > never use typedef. >> > But you do not do that. >> > >> > I fail to understand why you insist on using unsigned in place of a >> > more specific type, it is not the correct nor clean thing to do. >> >> >> It's not just me, it appears to be most of us. Can't you just step back >> at >> some point and be like "ok, I'll let the majority have their way"? > > I do not know what the majority prefers. What i see is that the > people objecting are always the same 3-4 people. And very often > they have no authorship or past activity in the code a patch is about. > At least none i could find quickly. How dare you speak like that about me? Do you think about yourself like holy cow in any aspect of FFmpeg, security or not.
On Sun, Jun 11, 2017 at 03:21:38PM +0200, Paul B Mahol wrote: > On 6/11/17, Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Fri, Jun 09, 2017 at 09:07:39PM -0400, Ronald S. Bultje wrote: > >> Hi, > >> > >> On Thu, Jun 8, 2017 at 8:57 PM, Michael Niedermayer > >> <michael@niedermayer.cc> > >> wrote: > >> > >> > On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote: > >> > > Hi, > >> > > > >> > > On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer > >> > <michael@niedermayer.cc> > >> > > wrote: > >> > > > >> > > > Signed value in > >> > > > Unsigned > >> > > > INTeger type > >> > > > >> > > [..] > >> > > > Both SUINT and unsigned should produce identical binaries > >> > > > >> > > This seems to go against the rule that code should be as simple as > >> > possible. > >> > > > >> > > Unsigned is simpler than SUINT if the outcome is the same. > >> > > >> > You can simply add the part of my mail here as awnser that you snipped > >> > away: > >> > > >> > "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." > >> > > >> > A type that avoids the undefinedness of signed but is semantically > >> > signed is correct, unsigned is not. > >> > > >> > If understandable code and maintainable code has no value to you, > >> > you would favour using single letter variables exclusivly and would > >> > never use typedef. > >> > But you do not do that. > >> > > >> > I fail to understand why you insist on using unsigned in place of a > >> > more specific type, it is not the correct nor clean thing to do. > >> > >> > >> It's not just me, it appears to be most of us. Can't you just step back > >> at > >> some point and be like "ok, I'll let the majority have their way"? > > > > I do not know what the majority prefers. What i see is that the > > people objecting are always the same 3-4 people. And very often > > they have no authorship or past activity in the code a patch is about. > > At least none i could find quickly. > > How dare you speak like that about me? About you ? It was not intended to be about you nor was it intended to insult anyone. Iam not sure why one would think otherwise. if i search now for onemda and mails matching SUINT there are only 5 matches, 2 or 3 of these have SUINT just in context IIUC. none is a reply to a patch from me in which you object due to SUINT. I did realize you arent in favor of the type but i didnt precive you as objecting to patches because of it. and I definitly tried to use unsigned instead of SUINT in the first place for code you wrote or maintain, if i made a mistake somewhere tell me and ill fix it. Also not complaining about it but people called the type a rootkit or part of a rootkit (thus kind of implying that i would add an exploit or rootkit), said i would ignore the majority and recommanded me to step back. (It wasnt you and it totally doesnt matter who did) but thats not exactly nice ... i should feel offended instead of you, no ? > > Do you think about yourself like holy cow in any aspect of FFmpeg, > security or not. no, certainly not. [...]
On 6/11/2017 10:21 AM, Paul B Mahol wrote: > On 6/11/17, Michael Niedermayer <michael@niedermayer.cc> wrote: >> On Fri, Jun 09, 2017 at 09:07:39PM -0400, Ronald S. Bultje wrote: >>> Hi, >>> >>> On Thu, Jun 8, 2017 at 8:57 PM, Michael Niedermayer >>> <michael@niedermayer.cc> >>> wrote: >>> >>>> On Thu, Jun 08, 2017 at 06:35:07PM -0400, Ronald S. Bultje wrote: >>>>> Hi, >>>>> >>>>> On Thu, Jun 8, 2017 at 6:10 PM, Michael Niedermayer >>>> <michael@niedermayer.cc> >>>>> wrote: >>>>> >>>>>> Signed value in >>>>>> Unsigned >>>>>> INTeger type >>>>> >>>>> [..] >>>>>> Both SUINT and unsigned should produce identical binaries >>>>> >>>>> This seems to go against the rule that code should be as simple as >>>> possible. >>>>> >>>>> Unsigned is simpler than SUINT if the outcome is the same. >>>> >>>> You can simply add the part of my mail here as awnser that you snipped >>>> away: >>>> >>>> "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." >>>> >>>> A type that avoids the undefinedness of signed but is semantically >>>> signed is correct, unsigned is not. >>>> >>>> If understandable code and maintainable code has no value to you, >>>> you would favour using single letter variables exclusivly and would >>>> never use typedef. >>>> But you do not do that. >>>> >>>> I fail to understand why you insist on using unsigned in place of a >>>> more specific type, it is not the correct nor clean thing to do. >>> >>> >>> It's not just me, it appears to be most of us. Can't you just step back >>> at >>> some point and be like "ok, I'll let the majority have their way"? >> >> I do not know what the majority prefers. What i see is that the >> people objecting are always the same 3-4 people. And very often >> they have no authorship or past activity in the code a patch is about. >> At least none i could find quickly. > > How dare you speak like that about me? > > Do you think about yourself like holy cow in any aspect of FFmpeg, > security or not. Please, can we all calm down? This is escalating way too much...
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; FFTComplex *tmpz; const int fft_size = (1 << s->nbits); int64_t accu; @@ -260,14 +260,14 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) { offset = ff_fft_offsets_lut[n] << 2; tmpz = z + offset; - tmp1 = tmpz[0].re + tmpz[1].re; - tmp5 = tmpz[2].re + tmpz[3].re; - tmp2 = tmpz[0].im + tmpz[1].im; - tmp6 = tmpz[2].im + tmpz[3].im; - tmp3 = tmpz[0].re - tmpz[1].re; - tmp8 = tmpz[2].im - tmpz[3].im; - tmp4 = tmpz[0].im - tmpz[1].im; - tmp7 = tmpz[2].re - tmpz[3].re; + tmp1 = tmpz[0].re + (SUINT)tmpz[1].re; + tmp5 = tmpz[2].re + (SUINT)tmpz[3].re; + tmp2 = tmpz[0].im + (SUINT)tmpz[1].im; + tmp6 = tmpz[2].im + (SUINT)tmpz[3].im; + tmp3 = tmpz[0].re - (SUINT)tmpz[1].re; + tmp8 = tmpz[2].im - (SUINT)tmpz[3].im; + tmp4 = tmpz[0].im - (SUINT)tmpz[1].im; + tmp7 = tmpz[2].re - (SUINT)tmpz[3].re; tmpz[0].re = tmp1 + tmp5; tmpz[2].re = tmp1 - tmp5; @@ -288,19 +288,19 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) { offset = ff_fft_offsets_lut[n] << 3; tmpz = z + offset; - tmp1 = tmpz[4].re + tmpz[5].re; - tmp3 = tmpz[6].re + tmpz[7].re; - tmp2 = tmpz[4].im + tmpz[5].im; - tmp4 = tmpz[6].im + tmpz[7].im; + tmp1 = tmpz[4].re + (SUINT)tmpz[5].re; + tmp3 = tmpz[6].re + (SUINT)tmpz[7].re; + tmp2 = tmpz[4].im + (SUINT)tmpz[5].im; + tmp4 = tmpz[6].im + (SUINT)tmpz[7].im; tmp5 = tmp1 + tmp3; tmp7 = tmp1 - tmp3; tmp6 = tmp2 + tmp4; tmp8 = tmp2 - tmp4; - tmp1 = tmpz[4].re - tmpz[5].re; - tmp2 = tmpz[4].im - tmpz[5].im; - tmp3 = tmpz[6].re - tmpz[7].re; - tmp4 = tmpz[6].im - tmpz[7].im; + tmp1 = tmpz[4].re - (SUINT)tmpz[5].re; + tmp2 = tmpz[4].im - (SUINT)tmpz[5].im; + tmp3 = tmpz[6].re - (SUINT)tmpz[7].re; + tmp4 = tmpz[6].im - (SUINT)tmpz[7].im; tmpz[4].re = tmpz[0].re - tmp5; tmpz[0].re = tmpz[0].re + tmp5; @@ -311,13 +311,13 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) { tmpz[6].im = tmpz[2].im + tmp7; tmpz[2].im = tmpz[2].im - tmp7; - accu = (int64_t)Q31(M_SQRT1_2)*(tmp1 + tmp2); + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp1 + tmp2); tmp5 = (int32_t)((accu + 0x40000000) >> 31); - accu = (int64_t)Q31(M_SQRT1_2)*(tmp3 - tmp4); + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp3 - tmp4); tmp7 = (int32_t)((accu + 0x40000000) >> 31); - accu = (int64_t)Q31(M_SQRT1_2)*(tmp2 - tmp1); + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp2 - tmp1); tmp6 = (int32_t)((accu + 0x40000000) >> 31); - accu = (int64_t)Q31(M_SQRT1_2)*(tmp3 + tmp4); + accu = (int64_t)Q31(M_SQRT1_2)*(int)(tmp3 + tmp4); tmp8 = (int32_t)((accu + 0x40000000) >> 31); tmp1 = tmp5 + tmp7; tmp3 = tmp5 - tmp7; @@ -348,10 +348,10 @@ static void fft_calc_c(FFTContext *s, FFTComplex *z) { offset = ff_fft_offsets_lut[n] << nbits; tmpz = z + offset; - tmp5 = tmpz[ n2].re + tmpz[n34].re; - tmp1 = tmpz[ n2].re - tmpz[n34].re; - tmp6 = tmpz[ n2].im + tmpz[n34].im; - tmp2 = tmpz[ n2].im - tmpz[n34].im; + tmp5 = tmpz[ n2].re + (SUINT)tmpz[n34].re; + tmp1 = tmpz[ n2].re - (SUINT)tmpz[n34].re; + tmp6 = tmpz[ n2].im + (SUINT)tmpz[n34].im; + tmp2 = tmpz[ n2].im - (SUINT)tmpz[n34].im; tmpz[ n2].re = tmpz[ 0].re - tmp5; tmpz[ 0].re = tmpz[ 0].re + tmp5;
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 <michael@niedermayer.cc> --- libavcodec/fft_template.c | 50 +++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 25 deletions(-)