From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.ffmpeg.org (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTPS id 361014CE8E for ; Mon, 11 Aug 2025 13:35:15 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTP id 7520968CB82; Mon, 11 Aug 2025 16:35:11 +0300 (EEST) Received: from vidala.pars.ee (vidala.pars.ee [116.203.72.101]) by ffbox0-bg.ffmpeg.org (Postfix) with ESMTPS id AB668687DA0 for ; Mon, 11 Aug 2025 16:35:04 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; s=202405r; d=lynne.ee; c=relaxed/relaxed; h=Message-ID:Date:Subject:To:From; t=1754919304; bh=DEG752aJTvhGJY2LoQcY39G VaLSdrjH12I2+dhRd4i0=; b=PZwFam1XJhbQ+25XdFNO1jqDvY1S53DdnQ/4JHdmL7MnKJyycZ AtG70OKYVQeyTlm/H+zJyVTPfsU8dGYwTPkRdVLFwkIExekmse7mzkiCsyqZSi/t2nC2c+lSlN2 RdoTncFHtz3E/x/e8mk3vjuR4TVSCB3ycnDUFz0nlB347spr8adSqkQn2QOP/GfquRFssaX+7PY sM+vUcnsDrEiioNepC22t5uaBKfFCRTL4mS4SZR1STPZFZvmg8wRIVRp1y5vE8d1tGnetZbTxwE Dag4uyy8gvMfCjrGUnwh/XIo55Kh3cX5iPI70SyJEAf3LbcGMKP4Qj1IRbCgYglYelg==; DKIM-Signature: v=1; a=ed25519-sha256; s=202405e; d=lynne.ee; c=relaxed/relaxed; h=Message-ID:Date:Subject:To:From; t=1754919304; bh=DEG752aJTvhGJY2LoQcY39G VaLSdrjH12I2+dhRd4i0=; b=7Mh7luB3SQFwgisXQcnWZKh2cdix0977RhjEvWJlL3n+xg8ar4 aZuYF0SPQ/ATedtGmRBMjEKt40x22H1YAqAg==; From: Lynne To: ffmpeg-devel@ffmpeg.org Date: Mon, 11 Aug 2025 22:34:47 +0900 Message-ID: <20250811133453.672337-1-dev@lynne.ee> X-Mailer: git-send-email 2.50.0 MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] aacdec_usac: use RefStruct to track unfinished extension buffers X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Lynne Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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".