| Message ID | 20200206134549.29792-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/cavsdsp: Fix invalid left shifts in cavs_idct8_add_c() | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
Michael Niedermayer: > Fixes: left shift of negative value -107 > Fixes: 20398/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CAVS_fuzzer-5725389278412800 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/cavsdsp.c | 52 ++++++++++++++++++++++---------------------- > 1 file changed, 26 insertions(+), 26 deletions(-) > > diff --git a/libavcodec/cavsdsp.c b/libavcodec/cavsdsp.c > index 90a67e910c..27413b9ee2 100644 > --- a/libavcodec/cavsdsp.c > +++ b/libavcodec/cavsdsp.c > @@ -201,20 +201,20 @@ static void cavs_idct8_add_c(uint8_t *dst, int16_t *block, ptrdiff_t stride) > src[0][0] += 8; > > for( i = 0; i < 8; i++ ) { > - const int a0 = 3*src[i][1] - (src[i][7]<<1); > - const int a1 = 3*src[i][3] + (src[i][5]<<1); > - const int a2 = (src[i][3]<<1) - 3*src[i][5]; > - const int a3 = (src[i][1]<<1) + 3*src[i][7]; > + const int a0 = 3*src[i][1] - (src[i][7]*2); > + const int a1 = 3*src[i][3] + (src[i][5]*2); > + const int a2 = (src[i][3]*2) - 3*src[i][5]; > + const int a3 = (src[i][1]*2) + 3*src[i][7]; > > - const int b4 = ((a0 + a1 + a3)<<1) + a1; > - const int b5 = ((a0 - a1 + a2)<<1) + a0; > - const int b6 = ((a3 - a2 - a1)<<1) + a3; > - const int b7 = ((a0 - a2 - a3)<<1) - a2; > + const int b4 = ((a0 + a1 + a3)*2) + a1; > + const int b5 = ((a0 - a1 + a2)*2) + a0; > + const int b6 = ((a3 - a2 - a1)*2) + a3; > + const int b7 = ((a0 - a2 - a3)*2) - a2; > > - const int a7 = (src[i][2]<<2) - 10*src[i][6]; > - const int a6 = (src[i][6]<<2) + 10*src[i][2]; > - const int a5 = ((src[i][0] - src[i][4]) << 3) + 4; > - const int a4 = ((src[i][0] + src[i][4]) << 3) + 4; > + const int a7 = (src[i][2]*4) - 10*src[i][6]; > + const int a6 = (src[i][6]*4) + 10*src[i][2]; > + const int a5 = ((src[i][0] - src[i][4]) * 8) + 4; > + const int a4 = ((src[i][0] + src[i][4]) * 8) + 4; > > const int b0 = a4 + a6; > const int b1 = a5 + a7; > @@ -231,20 +231,20 @@ static void cavs_idct8_add_c(uint8_t *dst, int16_t *block, ptrdiff_t stride) > src[i][7] = (b0 - b4) >> 3; > } > for( i = 0; i < 8; i++ ) { > - const int a0 = 3*src[1][i] - (src[7][i]<<1); > - const int a1 = 3*src[3][i] + (src[5][i]<<1); > - const int a2 = (src[3][i]<<1) - 3*src[5][i]; > - const int a3 = (src[1][i]<<1) + 3*src[7][i]; > - > - const int b4 = ((a0 + a1 + a3)<<1) + a1; > - const int b5 = ((a0 - a1 + a2)<<1) + a0; > - const int b6 = ((a3 - a2 - a1)<<1) + a3; > - const int b7 = ((a0 - a2 - a3)<<1) - a2; > - > - const int a7 = (src[2][i]<<2) - 10*src[6][i]; > - const int a6 = (src[6][i]<<2) + 10*src[2][i]; > - const int a5 = (src[0][i] - src[4][i]) << 3; > - const int a4 = (src[0][i] + src[4][i]) << 3; > + const int a0 = 3*src[1][i] - (src[7][i]*2); > + const int a1 = 3*src[3][i] + (src[5][i]*2); > + const int a2 = (src[3][i]*2) - 3*src[5][i]; > + const int a3 = (src[1][i]*2) + 3*src[7][i]; > + > + const int b4 = ((a0 + a1 + a3)*2) + a1; > + const int b5 = ((a0 - a1 + a2)*2) + a0; > + const int b6 = ((a3 - a2 - a1)*2) + a3; > + const int b7 = ((a0 - a2 - a3)*2) - a2; > + > + const int a7 = (src[2][i]*4) - 10*src[6][i]; > + const int a6 = (src[6][i]*4) + 10*src[2][i]; > + const int a5 = (src[0][i] - src[4][i]) * 8; > + const int a4 = (src[0][i] + src[4][i]) * 8; > > const int b0 = a4 + a6; > const int b1 = a5 + a7; > Duplicate of [1] which I consider more aesthetically pleasing (e.g. when rewriting "((a0 + a1 + a3)<<1) + a1" via multiplication, one can drop the outer parentheses, but you didn't). - Andreas [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-September/250681.html
On Thu, Feb 06, 2020 at 05:33:00PM +0000, Andreas Rheinhardt wrote: > Michael Niedermayer: > > Fixes: left shift of negative value -107 > > Fixes: 20398/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CAVS_fuzzer-5725389278412800 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/cavsdsp.c | 52 ++++++++++++++++++++++---------------------- > > 1 file changed, 26 insertions(+), 26 deletions(-) > > > > diff --git a/libavcodec/cavsdsp.c b/libavcodec/cavsdsp.c > > index 90a67e910c..27413b9ee2 100644 > > --- a/libavcodec/cavsdsp.c > > +++ b/libavcodec/cavsdsp.c > > @@ -201,20 +201,20 @@ static void cavs_idct8_add_c(uint8_t *dst, int16_t *block, ptrdiff_t stride) > > src[0][0] += 8; > > > > for( i = 0; i < 8; i++ ) { > > - const int a0 = 3*src[i][1] - (src[i][7]<<1); > > - const int a1 = 3*src[i][3] + (src[i][5]<<1); > > - const int a2 = (src[i][3]<<1) - 3*src[i][5]; > > - const int a3 = (src[i][1]<<1) + 3*src[i][7]; > > + const int a0 = 3*src[i][1] - (src[i][7]*2); > > + const int a1 = 3*src[i][3] + (src[i][5]*2); > > + const int a2 = (src[i][3]*2) - 3*src[i][5]; > > + const int a3 = (src[i][1]*2) + 3*src[i][7]; > > > > - const int b4 = ((a0 + a1 + a3)<<1) + a1; > > - const int b5 = ((a0 - a1 + a2)<<1) + a0; > > - const int b6 = ((a3 - a2 - a1)<<1) + a3; > > - const int b7 = ((a0 - a2 - a3)<<1) - a2; > > + const int b4 = ((a0 + a1 + a3)*2) + a1; > > + const int b5 = ((a0 - a1 + a2)*2) + a0; > > + const int b6 = ((a3 - a2 - a1)*2) + a3; > > + const int b7 = ((a0 - a2 - a3)*2) - a2; > > > > - const int a7 = (src[i][2]<<2) - 10*src[i][6]; > > - const int a6 = (src[i][6]<<2) + 10*src[i][2]; > > - const int a5 = ((src[i][0] - src[i][4]) << 3) + 4; > > - const int a4 = ((src[i][0] + src[i][4]) << 3) + 4; > > + const int a7 = (src[i][2]*4) - 10*src[i][6]; > > + const int a6 = (src[i][6]*4) + 10*src[i][2]; > > + const int a5 = ((src[i][0] - src[i][4]) * 8) + 4; > > + const int a4 = ((src[i][0] + src[i][4]) * 8) + 4; > > > > const int b0 = a4 + a6; > > const int b1 = a5 + a7; > > @@ -231,20 +231,20 @@ static void cavs_idct8_add_c(uint8_t *dst, int16_t *block, ptrdiff_t stride) > > src[i][7] = (b0 - b4) >> 3; > > } > > for( i = 0; i < 8; i++ ) { > > - const int a0 = 3*src[1][i] - (src[7][i]<<1); > > - const int a1 = 3*src[3][i] + (src[5][i]<<1); > > - const int a2 = (src[3][i]<<1) - 3*src[5][i]; > > - const int a3 = (src[1][i]<<1) + 3*src[7][i]; > > - > > - const int b4 = ((a0 + a1 + a3)<<1) + a1; > > - const int b5 = ((a0 - a1 + a2)<<1) + a0; > > - const int b6 = ((a3 - a2 - a1)<<1) + a3; > > - const int b7 = ((a0 - a2 - a3)<<1) - a2; > > - > > - const int a7 = (src[2][i]<<2) - 10*src[6][i]; > > - const int a6 = (src[6][i]<<2) + 10*src[2][i]; > > - const int a5 = (src[0][i] - src[4][i]) << 3; > > - const int a4 = (src[0][i] + src[4][i]) << 3; > > + const int a0 = 3*src[1][i] - (src[7][i]*2); > > + const int a1 = 3*src[3][i] + (src[5][i]*2); > > + const int a2 = (src[3][i]*2) - 3*src[5][i]; > > + const int a3 = (src[1][i]*2) + 3*src[7][i]; > > + > > + const int b4 = ((a0 + a1 + a3)*2) + a1; > > + const int b5 = ((a0 - a1 + a2)*2) + a0; > > + const int b6 = ((a3 - a2 - a1)*2) + a3; > > + const int b7 = ((a0 - a2 - a3)*2) - a2; > > + > > + const int a7 = (src[2][i]*4) - 10*src[6][i]; > > + const int a6 = (src[6][i]*4) + 10*src[2][i]; > > + const int a5 = (src[0][i] - src[4][i]) * 8; > > + const int a4 = (src[0][i] + src[4][i]) * 8; > > > > const int b0 = a4 + a6; > > const int b1 = a5 + a7; > > > Duplicate of [1] which I consider more aesthetically pleasing (e.g. > when rewriting "((a0 + a1 + a3)<<1) + a1" via multiplication, one can > drop the outer parentheses, but you didn't). > > - Andreas > > [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-September/250681.html will apply your variant Thanks [...]
diff --git a/libavcodec/cavsdsp.c b/libavcodec/cavsdsp.c index 90a67e910c..27413b9ee2 100644 --- a/libavcodec/cavsdsp.c +++ b/libavcodec/cavsdsp.c @@ -201,20 +201,20 @@ static void cavs_idct8_add_c(uint8_t *dst, int16_t *block, ptrdiff_t stride) src[0][0] += 8; for( i = 0; i < 8; i++ ) { - const int a0 = 3*src[i][1] - (src[i][7]<<1); - const int a1 = 3*src[i][3] + (src[i][5]<<1); - const int a2 = (src[i][3]<<1) - 3*src[i][5]; - const int a3 = (src[i][1]<<1) + 3*src[i][7]; + const int a0 = 3*src[i][1] - (src[i][7]*2); + const int a1 = 3*src[i][3] + (src[i][5]*2); + const int a2 = (src[i][3]*2) - 3*src[i][5]; + const int a3 = (src[i][1]*2) + 3*src[i][7]; - const int b4 = ((a0 + a1 + a3)<<1) + a1; - const int b5 = ((a0 - a1 + a2)<<1) + a0; - const int b6 = ((a3 - a2 - a1)<<1) + a3; - const int b7 = ((a0 - a2 - a3)<<1) - a2; + const int b4 = ((a0 + a1 + a3)*2) + a1; + const int b5 = ((a0 - a1 + a2)*2) + a0; + const int b6 = ((a3 - a2 - a1)*2) + a3; + const int b7 = ((a0 - a2 - a3)*2) - a2; - const int a7 = (src[i][2]<<2) - 10*src[i][6]; - const int a6 = (src[i][6]<<2) + 10*src[i][2]; - const int a5 = ((src[i][0] - src[i][4]) << 3) + 4; - const int a4 = ((src[i][0] + src[i][4]) << 3) + 4; + const int a7 = (src[i][2]*4) - 10*src[i][6]; + const int a6 = (src[i][6]*4) + 10*src[i][2]; + const int a5 = ((src[i][0] - src[i][4]) * 8) + 4; + const int a4 = ((src[i][0] + src[i][4]) * 8) + 4; const int b0 = a4 + a6; const int b1 = a5 + a7; @@ -231,20 +231,20 @@ static void cavs_idct8_add_c(uint8_t *dst, int16_t *block, ptrdiff_t stride) src[i][7] = (b0 - b4) >> 3; } for( i = 0; i < 8; i++ ) { - const int a0 = 3*src[1][i] - (src[7][i]<<1); - const int a1 = 3*src[3][i] + (src[5][i]<<1); - const int a2 = (src[3][i]<<1) - 3*src[5][i]; - const int a3 = (src[1][i]<<1) + 3*src[7][i]; - - const int b4 = ((a0 + a1 + a3)<<1) + a1; - const int b5 = ((a0 - a1 + a2)<<1) + a0; - const int b6 = ((a3 - a2 - a1)<<1) + a3; - const int b7 = ((a0 - a2 - a3)<<1) - a2; - - const int a7 = (src[2][i]<<2) - 10*src[6][i]; - const int a6 = (src[6][i]<<2) + 10*src[2][i]; - const int a5 = (src[0][i] - src[4][i]) << 3; - const int a4 = (src[0][i] + src[4][i]) << 3; + const int a0 = 3*src[1][i] - (src[7][i]*2); + const int a1 = 3*src[3][i] + (src[5][i]*2); + const int a2 = (src[3][i]*2) - 3*src[5][i]; + const int a3 = (src[1][i]*2) + 3*src[7][i]; + + const int b4 = ((a0 + a1 + a3)*2) + a1; + const int b5 = ((a0 - a1 + a2)*2) + a0; + const int b6 = ((a3 - a2 - a1)*2) + a3; + const int b7 = ((a0 - a2 - a3)*2) - a2; + + const int a7 = (src[2][i]*4) - 10*src[6][i]; + const int a6 = (src[6][i]*4) + 10*src[2][i]; + const int a5 = (src[0][i] - src[4][i]) * 8; + const int a4 = (src[0][i] + src[4][i]) * 8; const int b0 = a4 + a6; const int b1 = a5 + a7;
Fixes: left shift of negative value -107 Fixes: 20398/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CAVS_fuzzer-5725389278412800 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/cavsdsp.c | 52 ++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 26 deletions(-)