diff mbox series

[FFmpeg-devel,5/6] Revert "avcodec/er: remove check for fields"

Message ID 20230409142627.19820-5-michael@niedermayer.cc
State Accepted
Commit fa618f5f492f94d19dd8d32bcea084523fb4e2d8
Headers show
Series [FFmpeg-devel,1/6] avcodec/huffyuvdec: Fix undefined behavior with shift | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Michael Niedermayer April 9, 2023, 2:26 p.m. UTC 
Fixes: out of array write on x86-32
Fixes: 57825/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG2VIDEO_fuzzer-6094366187061248
Fixes: 57829/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG2VIDEO_fuzzer-4526419991724032

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
This reverts commit f7abe92bd7939b6aeeb2393fe141033e733305d4.
---
 libavcodec/error_resilience.c | 9 ++-------
 libavcodec/error_resilience.h | 1 -
 2 files changed, 2 insertions(+), 8 deletions(-)

Comments

Michael Niedermayer April 9, 2023, 9:15 p.m. UTC | #1 
On Sun, Apr 09, 2023 at 04:26:26PM +0200, Michael Niedermayer wrote:
> Fixes: out of array write on x86-32
> Fixes: 57825/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG2VIDEO_fuzzer-6094366187061248
> Fixes: 57829/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG2VIDEO_fuzzer-4526419991724032
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> This reverts commit f7abe92bd7939b6aeeb2393fe141033e733305d4.
> ---
>  libavcodec/error_resilience.c | 9 ++-------
>  libavcodec/error_resilience.h | 1 -
>  2 files changed, 2 insertions(+), 8 deletions(-)

Heres a backtrace for this btw

==7150==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xf62fe800 at pc 0x0899a5e5 bp 0xffc46c68 sp 0xffc46c60
WRITE of size 4 at 0xf62fe800 thread T0
    #0 0x899a5e4 in put_pixels8_8_c libavcodec/pel_template.c:78:1
    #1 0x8999ce3 in put_pixels16_8_c libavcodec/pel_template.c:78:1
    #2 0x82fafc4 in mpv_reconstruct_mb_internal libavcodec/mpv_reconstruct_mb_template.c:294:13
    #3 0x82fafc4 in ff_mpv_reconstruct_mb libavcodec/mpegvideo_dec.c:1023
    #4 0x82ae910 in mpeg_er_decode_mb libavcodec/mpeg_er.c:99:5
    #5 0x8752695 in guess_mv libavcodec/error_resilience.c:452:17
    #6 0x873cd02 in ff_er_frame_end libavcodec/error_resilience.c:1231:9
    #7 0x8273b04 in slice_end libavcodec/mpeg12dec.c:2027:9
    #8 0x8273b04 in decode_chunks libavcodec/mpeg12dec.c:2464
    #9 0x824455e in mpeg_decode_frame libavcodec/mpeg12dec.c:2813:11
    #10 0x818e36b in decode_simple_internal libavcodec/decode.c:287:15
    #11 0x816e31b in decode_simple_receive_frame libavcodec/decode.c:540:15
    #12 0x816e31b in decode_receive_frame_internal libavcodec/decode.c:560
    #13 0x816d7d6 in avcodec_send_packet libavcodec/decode.c:635:15
    #14 0x8129196 in LLVMFuzzerTestOneInput tools/target_dec_fuzzer.c:513:25
    #15 0x9060434 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned int) Fuzzer/./FuzzerLoop.cpp:495:13
    #16 0x9056298 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned int) Fuzzer/./FuzzerDriver.cpp:273:6
    #17 0x905a607 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned int)) Fuzzer/./FuzzerDriver.cpp:690:9
    #18 0x9056026 in main Fuzzer/./FuzzerMain.cpp:20:10
    #19 0xf7aaffa0 in __libc_start_main /build/glibc-IskIyT/glibc-2.27/csu/../csu/libc-start.c:310
    #20 0x8079541 in _start (tools/target_dec_mpeg2video_fuzzer+0x8079541)

0xf62fe800 is located 0 bytes to the right of 532480-byte region [0xf627c800,0xf62fe800)
allocated by thread T0 here:
    #0 0x80f0c5c in posix_memalign /b/swarming/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/asan/asan_malloc_linux.cc:226:3
    #1 0x8fcda9b in av_malloc libavutil/mem.c:105:9
    #2 0x8f290a9 in av_buffer_alloc libavutil/buffer.c:82:12
    #3 0x81319aa in fuzz_video_get_buffer tools/target_dec_fuzzer.c:131:25
    #4 0x81319aa in fuzz_get_buffer2 tools/target_dec_fuzzer.c:152
    #5 0x8186c1c in ff_get_buffer libavcodec/decode.c:1545:11
    #6 0x839c0d1 in ff_thread_get_ext_buffer libavcodec/pthread_frame.c:947:16
    #7 0x8b65f9f in alloc_frame_buffer libavcodec/mpegpicture.c:145:13
    #8 0x8b65f9f in ff_alloc_picture libavcodec/mpegpicture.c:272
    #9 0x82eab7d in alloc_picture libavcodec/mpegvideo_dec.c:245:12
    #10 0x82dd1c7 in ff_mpv_frame_start libavcodec/mpegvideo_dec.c:329:9
    #11 0x825eee7 in mpeg_field_start libavcodec/mpeg12dec.c:1580:20
    #12 0x825eee7 in decode_chunks libavcodec/mpeg12dec.c:2712
    #13 0x824455e in mpeg_decode_frame libavcodec/mpeg12dec.c:2813:11
    #14 0x818e36b in decode_simple_internal libavcodec/decode.c:287:15
    #15 0x816e31b in decode_simple_receive_frame libavcodec/decode.c:540:15
    #16 0x816e31b in decode_receive_frame_internal libavcodec/decode.c:560
    #17 0x816d7d6 in avcodec_send_packet libavcodec/decode.c:635:15
    #18 0x8129196 in LLVMFuzzerTestOneInput tools/target_dec_fuzzer.c:513:25
    #19 0x9060434 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned int) Fuzzer/./FuzzerLoop.cpp:495:13
    #20 0x9056298 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned int) Fuzzer/./FuzzerDriver.cpp:273:6
    #21 0x905a607 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned int)) Fuzzer/./FuzzerDriver.cpp:690:9
    #22 0x9056026 in main Fuzzer/./FuzzerMain.cpp:20:10
    #23 0xf7aaffa0 in __libc_start_main /build/glibc-IskIyT/glibc-2.27/csu/../csu/libc-start.c:310


[...]
Michael Niedermayer April 14, 2023, 12:37 a.m. UTC | #2 
On Sun, Apr 09, 2023 at 04:26:26PM +0200, Michael Niedermayer wrote:
> Fixes: out of array write on x86-32
> Fixes: 57825/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG2VIDEO_fuzzer-6094366187061248
> Fixes: 57829/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MPEG2VIDEO_fuzzer-4526419991724032
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> This reverts commit f7abe92bd7939b6aeeb2393fe141033e733305d4.
> ---
>  libavcodec/error_resilience.c | 9 ++-------
>  libavcodec/error_resilience.h | 1 -
>  2 files changed, 2 insertions(+), 8 deletions(-)

will apply this tomorrow (and also other parts of the patchset which have not
received any comments)

[...]
diff mbox series

Patch

diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
index bd7050062ce..2aa6f1d8640 100644
--- a/libavcodec/error_resilience.c
+++ b/libavcodec/error_resilience.c
@@ -805,7 +805,8 @@  void ff_er_frame_start(ERContext *s)
 static int er_supported(ERContext *s)
 {
     if(s->avctx->hwaccel && s->avctx->hwaccel->decode_slice           ||
-       !s->cur_pic.f
+       !s->cur_pic.f                                                  ||
+       s->cur_pic.field_picture
     )
         return 0;
     return 1;
@@ -907,12 +908,6 @@  void ff_er_frame_end(ERContext *s)
                           (s->avctx->skip_top + s->avctx->skip_bottom)) {
         return;
     }
-
-    if (!s->warned_fields && (s->cur_pic.field_picture || s->cur_pic.f->interlaced_frame)) {
-        av_log(s->avctx, AV_LOG_WARNING, "Error concealment is not fully implemented for field pictures.\n");
-        s->warned_fields = 1;
-    }
-
     linesize = s->cur_pic.f->linesize;
 
     if (   s->avctx->codec_id == AV_CODEC_ID_MPEG2VIDEO
diff --git a/libavcodec/error_resilience.h b/libavcodec/error_resilience.h
index 55efacaccc5..47cc8a4fc67 100644
--- a/libavcodec/error_resilience.h
+++ b/libavcodec/error_resilience.h
@@ -87,7 +87,6 @@  typedef struct ERContext {
                       int (*mv)[2][4][2],
                       int mb_x, int mb_y, int mb_intra, int mb_skipped);
     void *opaque;
-    int warned_fields;
 } ERContext;
 
 void ff_er_frame_start(ERContext *s);