[vlc-commits] contrib: ffmpeg: fix crash on use after free buffer in MPEG2 DXVA

[Steve Lhomme]

vlc/vlc-3.0 | branch: master | Steve Lhomme <robux4 at ycbcr.xyz> | Fri Feb 12 08:45:53 2021 +0100| [c35f6712bbf133f6a1223012e63a6161c4e79d47] | committer: Hugo Beauzée-Luyssen

contrib: ffmpeg: fix crash on use after free buffer in MPEG2 DXVA

Signed-off-by: Hugo Beauzée-Luyssen <hugo at beauzee.fr>
(cherry picked from commit f43cad135a6feccd6422114a00d9bc438e9d5af9)
Signed-off-by: Hugo Beauzée-Luyssen <hugo at beauzee.fr>

> http://git.videolan.org/gitweb.cgi/vlc/vlc-3.0.git/?a=commit;h=c35f6712bbf133f6a1223012e63a6161c4e79d47
---

 ...g12dec-don-t-call-hw-end_frame-when-start.patch | 36 ++++++++++++++++
 ...g12dec-don-t-end-a-slice-without-first_sl.patch | 49 ++++++++++++++++++++++
 contrib/src/ffmpeg/rules.mak                       |  2 +
 3 files changed, 87 insertions(+)

diff --git a/contrib/src/ffmpeg/0001-avcodec-mpeg12dec-don-t-call-hw-end_frame-when-start.patch b/contrib/src/ffmpeg/0001-avcodec-mpeg12dec-don-t-call-hw-end_frame-when-start.patch
new file mode 100644
index 0000000000..138ddd83e4
--- /dev/null
+++ b/contrib/src/ffmpeg/0001-avcodec-mpeg12dec-don-t-call-hw-end_frame-when-start.patch
@@ -0,0 +1,36 @@
+From 3ca2f6e24f4422f789e6483bb782052f7f91e4e4 Mon Sep 17 00:00:00 2001
+From: Steve Lhomme <robux4 at ycbcr.xyz>
+Date: Fri, 12 Feb 2021 08:20:56 +0100
+Subject: [PATCH 1/2] avcodec/mpeg12dec: don't call hw->end_frame when starting
+ second field decoding
+
+This call is unbalanced with a hwaccel->start_frame. It fixes some crashes
+because this call ends up using uninitialized memory. Decoding works as
+expected after this patch.
+---
+ libavcodec/mpeg12dec.c | 9 ---------
+ 1 file changed, 9 deletions(-)
+
+diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
+index ac5ac4bca4..88d10e9236 100644
+--- a/libavcodec/mpeg12dec.c
++++ b/libavcodec/mpeg12dec.c
+@@ -1674,15 +1674,6 @@ static int mpeg_field_start(MpegEncContext *s, const uint8_t *buf, int buf_size)
+             return AVERROR_INVALIDDATA;
+         }
+ 
+-        if (s->avctx->hwaccel &&
+-            (s->avctx->slice_flags & SLICE_FLAG_ALLOW_FIELD)) {
+-            if ((ret = s->avctx->hwaccel->end_frame(s->avctx)) < 0) {
+-                av_log(avctx, AV_LOG_ERROR,
+-                       "hardware accelerator failed to decode first field\n");
+-                return ret;
+-            }
+-        }
+-
+         for (i = 0; i < 4; i++) {
+             s->current_picture.f->data[i] = s->current_picture_ptr->f->data[i];
+             if (s->picture_structure == PICT_BOTTOM_FIELD)
+-- 
+2.27.0.windows.1
+
diff --git a/contrib/src/ffmpeg/0002-avcodec-mpeg12dec-don-t-end-a-slice-without-first_sl.patch b/contrib/src/ffmpeg/0002-avcodec-mpeg12dec-don-t-end-a-slice-without-first_sl.patch
new file mode 100644
index 0000000000..212343d251
--- /dev/null
+++ b/contrib/src/ffmpeg/0002-avcodec-mpeg12dec-don-t-end-a-slice-without-first_sl.patch
@@ -0,0 +1,49 @@
+From 3eb70589260b6eb0e37265933a15c737023dfd7e Mon Sep 17 00:00:00 2001
+From: Steve Lhomme <robux4 at ycbcr.xyz>
+Date: Fri, 12 Feb 2021 11:10:03 +0100
+Subject: [PATCH 2/2] avcodec/mpeg12dec: don't end a slice without first_slice
+
+If first_slice is set that means the first slice/field is not started yet. We
+should not end the slice. In particular calling hwaccel->end_frame may crash as
+we're ending a frame that was not started.
+
+We also need to reset first_slice once the slice_end is finished handling
+for this check to work.
+---
+ libavcodec/mpeg12dec.c | 20 +++++++++++++-------
+ 1 file changed, 13 insertions(+), 7 deletions(-)
+
+diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
+index 88d10e9236..fdf210fa1b 100644
+--- a/libavcodec/mpeg12dec.c
++++ b/libavcodec/mpeg12dec.c
+@@ -2477,13 +2477,19 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
+                         s2->er.error_count += s2->thread_context[i]->er.error_count;
+                 }
+ 
+-                ret = slice_end(avctx, picture);
+-                if (ret < 0)
+-                    return ret;
+-                else if (ret) {
+-                    // FIXME: merge with the stuff in mpeg_decode_slice
+-                    if (s2->last_picture_ptr || s2->low_delay)
+-                        *got_output = 1;
++                if (s->first_slice) // not started yet. don't end it
++                    ret = 0;
++                else {
++                    ret = slice_end(avctx, picture);
++                    if (ret < 0)
++                        return ret;
++                    else if (ret) {
++                        // FIXME: merge with the stuff in mpeg_decode_slice
++                        if (s2->last_picture_ptr || s2->low_delay)
++                            *got_output = 1;
++                    }
++                    // slice ended, don't end it again later
++                    s->first_slice = 1;
+                 }
+             }
+             s2->pict_type = 0;
+-- 
+2.27.0.windows.1
+
diff --git a/contrib/src/ffmpeg/rules.mak b/contrib/src/ffmpeg/rules.mak
index 4db97585ea..7966c653d6 100644
--- a/contrib/src/ffmpeg/rules.mak
+++ b/contrib/src/ffmpeg/rules.mak
@@ -245,6 +245,8 @@ ifdef USE_FFMPEG
 	$(APPLY) $(SRC)/ffmpeg/0001-avcodec-hevcdec-set-the-SEI-parameters-early-on-the-.patch
 	$(APPLY) $(SRC)/ffmpeg/0001-avcodec-h264_slice-set-the-SEI-parameters-early-on-t.patch
 	$(APPLY) $(SRC)/ffmpeg/0001-avcodec-vp9-add-profile-2-10-bit-DXVA2-D3D11-decodin.patch
+	$(APPLY) $(SRC)/ffmpeg/0001-avcodec-mpeg12dec-don-t-call-hw-end_frame-when-start.patch
+	$(APPLY) $(SRC)/ffmpeg/0002-avcodec-mpeg12dec-don-t-end-a-slice-without-first_sl.patch
 endif
 ifdef USE_LIBAV
 	$(APPLY) $(SRC)/ffmpeg/libav_gsm.patch