Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] aacdec_usac: use RefStruct to track unfinished extension buffers
@ 2025-08-11 13:34 Lynne
  0 siblings, 0 replies; only message in thread
From: Lynne @ 2025-08-11 13:34 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Lynne

Extensions in AAC USAC can be stored across multiple frames (mainly to keep CBR compliance).
This means that we need to reallocate a buffer when new data is received, accumulate the bitstream data,
and so on until the end of extension flag is signalled and the extension can be decoded.

This is made more complicated by the way in which the AAC channel layout switching is performed.
After decades of evolution, our AAC decoder evolved to double-buffer its entire configuration.
All changes are buffered, verified, and applied, on a per-frame basis if required, in often
random order.

Since we allocate the extension data on heap, this means that if configuration is applied,
in order to avoid double-freeing, we have to keep track of what we've allocated.

It should be noted that extensions which are spread in multiple frames are generally rare,
so an optimization to introduce av_refstruct_realloc() wouldn't generally be useful across the codebase.
Therefore, a copy is good enough for now.

Fixes: double free
Fixes: 393523547/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-6740617236905984

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
---
 libavcodec/aac/aacdec.c      | 28 +++++++++++++++++++++++++---
 libavcodec/aac/aacdec.h      |  4 ++--
 libavcodec/aac/aacdec_usac.c | 31 ++++++++++++++++++-------------
 3 files changed, 45 insertions(+), 18 deletions(-)

diff --git a/libavcodec/aac/aacdec.c b/libavcodec/aac/aacdec.c
index 6a2aa9dc8e..bfa60708be 100644
--- a/libavcodec/aac/aacdec.c
+++ b/libavcodec/aac/aacdec.c
@@ -62,6 +62,7 @@
 #include "libavutil/opt.h"
 #include "libavutil/tx.h"
 #include "libavutil/version.h"
+#include "libavutil/refstruct.h"
 
 /*
  * supported tools
@@ -421,6 +422,26 @@ static uint64_t sniff_channel_order(uint8_t (*layout_map)[3], int tags)
     return layout;
 }
 
+static void copy_oc(OutputConfiguration *dst, OutputConfiguration *src)
+{
+    uint8_t *elem_exts[64];
+
+    for (int i = 0; i < src->usac.nb_elems; i++) {
+        AACUsacElemConfig *src_e = &src->usac.elems[i];
+        AACUsacElemConfig *dst_e = &dst->usac.elems[i];
+
+        av_refstruct_replace(&dst_e->ext.pl_buf, src_e->ext.pl_buf);
+        elem_exts[i] = dst_e->ext.pl_buf;
+    }
+
+    *dst = *src;
+
+    for (int i = 0; i < src->usac.nb_elems; i++) {
+        AACUsacElemConfig *dst_e = &dst->usac.elems[i];
+        dst_e->ext.pl_buf = elem_exts[i];
+    }
+}
+
 /**
  * Save current output configuration if and only if it has been locked.
  */
@@ -429,7 +450,7 @@ static int push_output_configuration(AACDecContext *ac)
     int pushed = 0;
 
     if (ac->oc[1].status == OC_LOCKED || ac->oc[0].status == OC_NONE) {
-        ac->oc[0] = ac->oc[1];
+        copy_oc(&ac->oc[0], &ac->oc[1]);
         pushed = 1;
     }
     ac->oc[1].status = OC_NONE;
@@ -443,7 +464,8 @@ static int push_output_configuration(AACDecContext *ac)
 static void pop_output_configuration(AACDecContext *ac)
 {
     if (ac->oc[1].status != OC_LOCKED && ac->oc[0].status != OC_NONE) {
-        ac->oc[1] = ac->oc[0];
+        copy_oc(&ac->oc[1], &ac->oc[0]);
+
         ac->avctx->ch_layout = ac->oc[1].ch_layout;
         ff_aac_output_configure(ac, ac->oc[1].layout_map, ac->oc[1].layout_map_tags,
                                 ac->oc[1].status, 0);
@@ -1107,7 +1129,7 @@ static av_cold int decode_close(AVCodecContext *avctx)
         AACUSACConfig *usac = &oc->usac;
         for (int j = 0; j < usac->nb_elems; j++) {
             AACUsacElemConfig *ec = &usac->elems[j];
-            av_freep(&ec->ext.pl_data);
+            av_refstruct_unref(&ec->ext.pl_buf);
         }
 
         av_channel_layout_uninit(&ac->oc[i].ch_layout);
diff --git a/libavcodec/aac/aacdec.h b/libavcodec/aac/aacdec.h
index e5a79a7139..b3763fdccc 100644
--- a/libavcodec/aac/aacdec.h
+++ b/libavcodec/aac/aacdec.h
@@ -344,7 +344,7 @@ typedef struct AACUsacElemConfig {
         uint8_t payload_frag;
         uint32_t default_len;
         uint32_t pl_data_offset;
-        uint8_t *pl_data;
+        uint8_t *pl_buf;
     } ext;
 } AACUsacElemConfig;
 
@@ -353,7 +353,7 @@ typedef struct AACUSACConfig {
     uint16_t core_frame_len;
     uint16_t stream_identifier;
 
-    AACUsacElemConfig elems[64];
+    AACUsacElemConfig elems[MAX_ELEM_ID];
     int nb_elems;
 
     struct {
diff --git a/libavcodec/aac/aacdec_usac.c b/libavcodec/aac/aacdec_usac.c
index e03e6e015f..cd933674b0 100644
--- a/libavcodec/aac/aacdec_usac.c
+++ b/libavcodec/aac/aacdec_usac.c
@@ -24,12 +24,13 @@
 #include "aacdec_ac.h"
 
 #include "libavcodec/aacsbr.h"
-
 #include "libavcodec/aactab.h"
-#include "libavutil/mem.h"
 #include "libavcodec/mpeg4audio.h"
 #include "libavcodec/unary.h"
 
+#include "libavutil/mem.h"
+#include "libavutil/refstruct.h"
+
 /* Number of scalefactor bands per complex prediction band, equal to 2. */
 #define SFB_PER_PRED_BAND 2
 
@@ -1574,7 +1575,6 @@ static int parse_audio_preroll(AACDecContext *ac, GetBitContext *gb)
 static int parse_ext_ele(AACDecContext *ac, AACUsacElemConfig *e,
                          GetBitContext *gb)
 {
-    uint8_t *tmp;
     uint8_t pl_frag_start = 1;
     uint8_t pl_frag_end = 1;
     uint32_t len;
@@ -1601,18 +1601,23 @@ static int parse_ext_ele(AACDecContext *ac, AACUsacElemConfig *e,
     if (pl_frag_start)
         e->ext.pl_data_offset = 0;
 
-    /* If an extension starts and ends this packet, we can directly use it */
+    /* If an extension starts and ends this packet, we can directly use it below.
+     * Otherwise, we have to copy it to a buffer and accumulate it. */
     if (!(pl_frag_start && pl_frag_end)) {
-        tmp = av_realloc(e->ext.pl_data, e->ext.pl_data_offset + len);
-        if (!tmp) {
-            av_free(e->ext.pl_data);
-            return AVERROR(ENOMEM);
-        }
-        e->ext.pl_data = tmp;
+         /* Reallocate the data */
+         uint8_t *tmp_buf = av_refstruct_alloc_ext(e->ext.pl_data_offset + len,
+                                                   AV_REFSTRUCT_FLAG_NO_ZEROING,
+                                                   NULL, NULL);
+         if (!tmp_buf)
+             return AVERROR(ENOMEM);
+
+        memcpy(tmp_buf, e->ext.pl_buf, e->ext.pl_data_offset);
+        av_refstruct_unref(&e->ext.pl_buf);
+        e->ext.pl_buf = tmp_buf;
 
         /* Readout data to a buffer */
         for (int i = 0; i < len; i++)
-            e->ext.pl_data[e->ext.pl_data_offset + i] = get_bits(gb, 8);
+            e->ext.pl_buf[e->ext.pl_data_offset + i] = get_bits(gb, 8);
     }
 
     e->ext.pl_data_offset += len;
@@ -1624,7 +1629,7 @@ static int parse_ext_ele(AACDecContext *ac, AACUsacElemConfig *e,
         GetBitContext *gb2 = gb;
         GetBitContext gbc;
         if (!(pl_frag_start && pl_frag_end)) {
-            ret = init_get_bits8(&gbc, e->ext.pl_data, pl_len);
+            ret = init_get_bits8(&gbc, e->ext.pl_buf, pl_len);
             if (ret < 0)
                 return ret;
 
@@ -1642,7 +1647,7 @@ static int parse_ext_ele(AACDecContext *ac, AACUsacElemConfig *e,
             /* This should never happen */
             av_assert0(0);
         }
-        av_freep(&e->ext.pl_data);
+        av_refstruct_unref(&e->ext.pl_buf);
         if (ret < 0)
             return ret;
 
-- 
2.50.0
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2025-08-11 13:35 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-11 13:34 [FFmpeg-devel] [PATCH] aacdec_usac: use RefStruct to track unfinished extension buffers Lynne

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git