diff mbox series

[FFmpeg-devel,08/12] avcodec/decode: Fix leaks upon subtitle decoding errors

Message ID AM7PR03MB666039BDDEFC04A265514FBB8F729@AM7PR03MB6660.eurprd03.prod.outlook.com
State Accepted
Commit cee04cbfe1bf77db5a5bb4ade786573f08a586fc
Headers show
Series [FFmpeg-devel,01/12] avcodec/h2645_parse: Remove H2645NAL.rbsp_buffer | expand

Checks

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

Commit Message

Andreas Rheinhardt Dec. 11, 2021, 6:40 p.m. UTC 
Up until now, various subtitle decoders have not cleaned up
the AVSubtitle on error; this task must not be left to the user
because the documentation explicitly states that the AVSubtitle
"must be freed with avsubtitle_free if *got_sub_ptr is set"
(which it isn't on error).
Leaks happen upon failure in ff_ass_add_rect() or in
ass_decode_frame(); freeing generically also allows to remove
now redundant freeing code in pgssubdec and dvbsubdec.
While just at it, also reset got_sub_ptr generically on error.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/decode.c    | 13 ++++++++-----
 libavcodec/dvbsubdec.c |  2 --
 libavcodec/pgssubdec.c |  5 +----
 3 files changed, 9 insertions(+), 11 deletions(-)
diff mbox series

Patch

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 52bf5dcd33..afe2c0c3c1 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -834,8 +834,14 @@  int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
             sub->pts = av_rescale_q(avpkt->pts,
                                     avctx->pkt_timebase, AV_TIME_BASE_Q);
         ret = avctx->codec->decode(avctx, sub, got_sub_ptr, pkt);
-        av_assert1((ret >= 0) >= !!*got_sub_ptr &&
-                   !!*got_sub_ptr >= !!sub->num_rects);
+        if (pkt == avci->buffer_pkt) // did we recode?
+            av_packet_unref(avci->buffer_pkt);
+        if (ret < 0) {
+            *got_sub_ptr = 0;
+            avsubtitle_free(sub);
+            return ret;
+        }
+        av_assert1(!sub->num_rects || *got_sub_ptr);
 
         if (sub->num_rects && !sub->end_display_time && avpkt->duration &&
             avctx->pkt_timebase.num) {
@@ -863,9 +869,6 @@  int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
 
         if (*got_sub_ptr)
             avctx->frame_number++;
-
-        if (pkt == avci->buffer_pkt) // did we recode?
-            av_packet_unref(avci->buffer_pkt);
     }
 
     return ret;
diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
index 81ccaf4c57..bc741a1de6 100644
--- a/libavcodec/dvbsubdec.c
+++ b/libavcodec/dvbsubdec.c
@@ -1715,8 +1715,6 @@  static int dvbsub_decode(AVCodecContext *avctx,
 
 end:
     if (ret < 0) {
-        *got_sub_ptr = 0;
-        avsubtitle_free(sub);
         return ret;
     } else {
         if (ctx->compute_edt == 1)
diff --git a/libavcodec/pgssubdec.c b/libavcodec/pgssubdec.c
index 8a200aa01b..bdd20c914b 100644
--- a/libavcodec/pgssubdec.c
+++ b/libavcodec/pgssubdec.c
@@ -667,11 +667,8 @@  static int decode(AVCodecContext *avctx, void *data, int *got_sub_ptr,
             break;
         }
         if (ret < 0 && (ret == AVERROR(ENOMEM) ||
-                        avctx->err_recognition & AV_EF_EXPLODE)) {
-            avsubtitle_free(data);
-            *got_sub_ptr = 0;
+                        avctx->err_recognition & AV_EF_EXPLODE))
             return ret;
-        }
 
         buf += segment_length;
     }