diff mbox series

[FFmpeg-devel,4/4] avformat/avidec: Fix memleak with embedded GAB2 subtitles

Message ID 20200327095557.24069-4-andreas.rheinhardt@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel,1/4] avformat/avidec: Don't reimplement ff_free_stream()
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt March 27, 2020, 9:55 a.m. UTC
The code for GAB2 subtitles predates refcounting AVPackets. So in order
to transfer the ownership of a packet's data pkt->data was simply stored
and the packet zeroed; in the end (i.e. in the read_close-function) this
data was then simply freed with av_freep(). This of course leads to a leak
of an AVBufferRef and an AVBuffer. It has been fixed by keeping and
eventually unreferencing the packet's buf instead.

Additionally, the packet is now reset via av_packet_unref().

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
This is actually not the only thing wrong with these GAB2 subtitles; but
it is the only issue that exists on success.

(Actually, both the srt and ass demuxer read the whole file when reading
the header and store the result in a subtitles queue; so keeping the
data is actually unnecessary.)

 libavformat/avidec.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/avidec.c b/libavformat/avidec.c
index 87298513c2..5fc3e01aa9 100644
--- a/libavformat/avidec.c
+++ b/libavformat/avidec.c
@@ -60,7 +60,7 @@  typedef struct AVIStream {
 
     AVFormatContext *sub_ctx;
     AVPacket sub_pkt;
-    uint8_t *sub_buffer;
+    AVBufferRef *sub_buffer;
 
     int64_t seek_pos;
 } AVIStream;
@@ -1116,8 +1116,9 @@  static int read_gab2_sub(AVFormatContext *s, AVStream *st, AVPacket *pkt)
             time_base = ast->sub_ctx->streams[0]->time_base;
             avpriv_set_pts_info(st, 64, time_base.num, time_base.den);
         }
-        ast->sub_buffer = pkt->data;
-        memset(pkt, 0, sizeof(*pkt));
+        ast->sub_buffer = pkt->buf;
+        pkt->buf = NULL;
+        av_packet_unref(pkt);
         return 1;
 
 error:
@@ -1909,7 +1910,7 @@  static int avi_read_close(AVFormatContext *s)
                 av_freep(&ast->sub_ctx->pb);
                 avformat_close_input(&ast->sub_ctx);
             }
-            av_freep(&ast->sub_buffer);
+            av_buffer_unref(&ast->sub_buffer);
             av_packet_unref(&ast->sub_pkt);
         }
     }