diff mbox series

[FFmpeg-devel] avformat/pp_bnk: Fix memleaks when reading non-stereo tracks

Message ID 20210320070801.94890-1-andreas.rheinhardt@gmail.com
State Accepted
Commit 8a73313412eeafcfa5afa45f39f65f2581ba3bbc
Headers show
Series [FFmpeg-devel] avformat/pp_bnk: Fix memleaks when reading non-stereo tracks | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt March 20, 2021, 7:08 a.m. UTC
Commit 6973df112275c8ea4af0bf3cb1338baecc1d06b3 added support
for music tracks by outputting its two containing tracks
together in one packet. But the actual data is not contiguous
in the file and therefore one can't simply use av_get_packet()
(which has been used before) for it. Therefore the packet was
now allocated via av_new_packet() and read via avio_read();
and this is also for non-music files.

This causes problems because one can now longer rely on things
done automatically by av_get_packet(): It automatically freed
the packet in case of errors; this lead to memleaks in several
FATE-tests covering this demuxer. Furthermore, in case the data
read is less than the data desired, the returned packet was not
zero-allocated (the packet's padding was uninitialized);
for music files the actual data could even be uninitialized.

The former problems are fixed by using av_get_packet() for
non-music files; the latter problem is handled by erroring out
unless both tracks could be fully read.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/pp_bnk.c | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/pp_bnk.c b/libavformat/pp_bnk.c
index 4aa77c1c9d..5c89295d27 100644
--- a/libavformat/pp_bnk.c
+++ b/libavformat/pp_bnk.c
@@ -265,28 +265,24 @@  static int pp_bnk_read_packet(AVFormatContext *s, AVPacket *pkt)
 
         size = FFMIN(trk->data_size - trk->bytes_read, PP_BNK_MAX_READ_SIZE);
 
-        if (!ctx->is_music)
-            ret = av_new_packet(pkt, size);
-        else if (ctx->current_track == 0)
-            ret = av_new_packet(pkt, size * 2);
-        else
-            ret = 0;
-
-        if (ret < 0)
-            return ret;
-
-        if (ctx->is_music)
+        if (!ctx->is_music) {
+            ret = av_get_packet(s->pb, pkt, size);
+            if (ret == AVERROR_EOF) {
+                /* If we've hit EOF, don't attempt this track again. */
+                trk->data_size = trk->bytes_read;
+                continue;
+            }
+        } else {
+            if (!pkt->data && (ret = av_new_packet(pkt, size * 2)) < 0)
+                return ret;
             ret = avio_read(s->pb, pkt->data + size * ctx->current_track, size);
-        else
-            ret = avio_read(s->pb, pkt->data, size);
-
-        if (ret == AVERROR_EOF) {
-            /* If we've hit EOF, don't attempt this track again. */
-            trk->data_size = trk->bytes_read;
-            continue;
-        } else if (ret < 0) {
-            return ret;
+            if (ret >= 0 && ret != size) {
+                /* Only return stereo packets if both tracks could be read. */
+                ret = AVERROR_EOF;
+            }
         }
+        if (ret < 0)
+            return ret;
 
         trk->bytes_read    += ret;
         pkt->flags         &= ~AV_PKT_FLAG_CORRUPT;
@@ -298,8 +294,6 @@  static int pp_bnk_read_packet(AVFormatContext *s, AVPacket *pkt)
                 continue;
 
             pkt->stream_index = 0;
-        } else {
-            pkt->size = ret;
         }
 
         ctx->current_track++;