diff mbox series

[FFmpeg-devel,09/27] avformat/cafenc: Don't segfault upon allocation error

Message ID AM7PR03MB66603FFAE6193E1C78CE7B328FA39@AM7PR03MB6660.eurprd03.prod.outlook.com
State Accepted
Commit 19a6b51fe61b915b734319b5d917192108df8188
Headers show
Series [FFmpeg-devel,01/27] avformat/astenc: Simplify writing padding | expand

Commit Message

Andreas Rheinhardt Sept. 23, 2021, 3:28 p.m. UTC 
If an array for the packet sizes could not be successfully reallocated
when writing a packet, the CAF muxer frees said array, but does not
reset the number of valid bytes. As a result, when the trailer is
written later, avio_write tries to read that many bytes from NULL,
which segfaults.

Fix this by not freeing the array in case of error; also, postpone
writing the packet data after having successfully (re)allocated the
array, so that even on allocation error the file can be correctly
finalized.

Also remove an unnecessary resetting of the number of size entries
used at the end.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/cafenc.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)
diff mbox series

Patch

diff --git a/libavformat/cafenc.c b/libavformat/cafenc.c
index 622ae14264..b17d2397e9 100644
--- a/libavformat/cafenc.c
+++ b/libavformat/cafenc.c
@@ -210,21 +210,18 @@  static int caf_write_packet(AVFormatContext *s, AVPacket *pkt)
 {
     CAFContext *caf = s->priv_data;
 
-    avio_write(s->pb, pkt->data, pkt->size);
     if (!s->streams[0]->codecpar->block_align) {
-        void *pkt_sizes = caf->pkt_sizes;
+        void *pkt_sizes;
         int i, alloc_size = caf->size_entries_used + 5U;
-        if (alloc_size < 0) {
-            caf->pkt_sizes = NULL;
-        } else {
-            caf->pkt_sizes = av_fast_realloc(caf->pkt_sizes,
-                                             &caf->size_buffer_size,
-                                             alloc_size);
-        }
-        if (!caf->pkt_sizes) {
-            av_free(pkt_sizes);
+        if (alloc_size < 0)
+            return AVERROR(ERANGE);
+
+        pkt_sizes = av_fast_realloc(caf->pkt_sizes,
+                                    &caf->size_buffer_size,
+                                    alloc_size);
+        if (!pkt_sizes)
             return AVERROR(ENOMEM);
-        }
+        caf->pkt_sizes = pkt_sizes;
         for (i = 4; i > 0; i--) {
             unsigned top = pkt->size >> i * 7;
             if (top)
@@ -233,6 +230,7 @@  static int caf_write_packet(AVFormatContext *s, AVPacket *pkt)
         caf->pkt_sizes[caf->size_entries_used++] = pkt->size & 127;
         caf->packets++;
     }
+    avio_write(s->pb, pkt->data, pkt->size);
     return 0;
 }
 
@@ -263,7 +261,6 @@  static int caf_write_trailer(AVFormatContext *s)
             avio_wb32(pb, 0); ///< mPrimingFrames
             avio_wb32(pb, 0); ///< mRemainderFrames
             avio_write(pb, caf->pkt_sizes, caf->size_entries_used);
-            caf->size_buffer_size = 0;
         }
     }
     av_freep(&caf->pkt_sizes);