PR #25459 opened by concretecactus URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25459 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25459.patch
Remove assignment chains that can target the same memory location, as there is no sequence point between separate assignments, and can cause undefined behavior. Refactor multiple get_bits and bytestream_get invocations in one expression. The compiler can evaluate the get_bits calls in a different order, which is a potential bug. (h263dec.c) h263_get_format changes avctx->pix_fmt, so a comparison with the same value can result in unexpected behavior. >From 9e2cca0cea251a94606adacf5fb5402769a2b0b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81ron=20H=C3=A1rn=C3=A1si?= <[email protected]> Date: Sun, 25 Oct 2026 20:17:32 +0100 Subject: [PATCH] fix(av): Fix unsequenced variable accesses Remove assignment chains that can target the same memory location, as there is no sequence point between separate assignments, and can cause undefined behavior. Refactor multiple get_bits and bytestream_get invocations in one expression. The compiler can evaluate the get_bits calls in a different order, which is a potential bug. (h263dec.c) h263_get_format changes avctx->pix_fmt, so a comparison with the same value can result in unexpected behavior. --- libavcodec/atrac1.c | 7 ++++--- libavcodec/get_buffer.c | 3 ++- libavcodec/h263dec.c | 3 ++- libavcodec/hcadec.c | 6 ++++-- libavfilter/af_adeclick.c | 7 +++++-- libavfilter/framepool.c | 7 +++++-- libavfilter/vf_xbr.c | 12 ++++++------ libavformat/apngdec.c | 6 +++++- libavformat/takdec.c | 7 +++++-- libavutil/frame.c | 7 +++++-- 10 files changed, 43 insertions(+), 22 deletions(-) diff --git a/libavcodec/atrac1.c b/libavcodec/atrac1.c index 2625b8f87d3..314cbb76074 100644 --- a/libavcodec/atrac1.c +++ b/libavcodec/atrac1.c @@ -198,6 +198,7 @@ static int at1_unpack_dequant(GetBitContext* gb, AT1SUCtx* su, int bits_used, band_num, bfu_num, i; uint8_t idwls[AT1_MAX_BFU]; ///< the word length indexes for each BFU uint8_t idsfs[AT1_MAX_BFU]; ///< the scalefactor indexes for each BFU + uint8_t bfu_amount2, bfu_amount3; /* parse the info byte (2nd byte) telling how much BFUs were coded */ su->num_bfus = bfu_amount_tab1[get_bits(gb, 3)]; @@ -205,9 +206,9 @@ static int at1_unpack_dequant(GetBitContext* gb, AT1SUCtx* su, /* calc number of consumed bits: num_BFUs * (idwl(4bits) + idsf(6bits)) + log2_block_count(8bits) + info_byte(8bits) + info_byte_copy(8bits) + log2_block_count_copy(8bits) */ - bits_used = su->num_bfus * 10 + 32 + - bfu_amount_tab2[get_bits(gb, 2)] + - (bfu_amount_tab3[get_bits(gb, 3)] << 1); + bfu_amount2 = bfu_amount_tab2[get_bits(gb, 2)]; + bfu_amount3 = bfu_amount_tab3[get_bits(gb, 3)]; + bits_used = su->num_bfus * 10 + 32 + bfu_amount2 + (bfu_amount3 << 1); /* get word length index (idwl) for each BFU */ for (i = 0; i < su->num_bfus; i++) diff --git a/libavcodec/get_buffer.c b/libavcodec/get_buffer.c index 1e0cfc21da4..20104fc549a 100644 --- a/libavcodec/get_buffer.c +++ b/libavcodec/get_buffer.c @@ -196,7 +196,8 @@ static int audio_get_buffer(AVCodecContext *avctx, AVFrame *frame) frame->buf[i] = av_buffer_pool_get(pool->pools[0]); if (!frame->buf[i]) goto fail; - frame->extended_data[i] = frame->data[i] = frame->buf[i]->data; + frame->extended_data[i] = frame->buf[i]->data; + frame->data[i] = frame->buf[i]->data; } for (i = 0; i < frame->nb_extended_buf; i++) { frame->extended_buf[i] = av_buffer_pool_get(pool->pools[0]); diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c index 9fc30f7b1b2..926a6ce1c92 100644 --- a/libavcodec/h263dec.c +++ b/libavcodec/h263dec.c @@ -537,7 +537,8 @@ int ff_h263_decode_frame(AVCodecContext *avctx, AVFrame *pict, if ((ret = ff_mpv_common_frame_size_change(s))) return ret; - if (avctx->pix_fmt != h263_get_format(avctx)) { + enum AVPixelFormat pix_fmt = h263_get_format(avctx); + if (avctx->pix_fmt != pix_fmt) { av_log(avctx, AV_LOG_ERROR, "format change not supported\n"); avctx->pix_fmt = AV_PIX_FMT_NONE; return AVERROR_UNKNOWN; diff --git a/libavcodec/hcadec.c b/libavcodec/hcadec.c index 2eac70b60ff..9e03e7715ca 100644 --- a/libavcodec/hcadec.c +++ b/libavcodec/hcadec.c @@ -528,7 +528,7 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame, int *got_frame_ptr, AVPacket *avpkt) { HCAContext *c = avctx->priv_data; - int ch, offset = 0, ret, packed_noise_level; + int ch, offset = 0, ret, noise_level1, noise_level2, packed_noise_level; GetBitContext gb0, *const gb = &gb0; float **samples; @@ -581,7 +581,9 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *frame, return ret; samples = (float **)frame->extended_data; - packed_noise_level = (get_bits(gb, 9) << 8) - get_bits(gb, 7); + noise_level1 = get_bits(gb, 9); + noise_level2 = get_bits(gb, 7); + packed_noise_level = (noise_level1 << 8) - noise_level2; for (ch = 0; ch < avctx->ch_layout.nb_channels; ch++) unpack(c, &c->ch[ch], gb, c->hfr_group_count, packed_noise_level, c->ath); diff --git a/libavfilter/af_adeclick.c b/libavfilter/af_adeclick.c index 198e7fab633..c6ef3fc894b 100644 --- a/libavfilter/af_adeclick.c +++ b/libavfilter/af_adeclick.c @@ -416,9 +416,12 @@ static int interpolation(DeclickChannel *c, const double *src, int ar_order, for (j = i; j < nb_errors; j++) { if (abs(index[j] - index[i]) <= ar_order) { - matrix[j * nb_errors + i] = matrix[im + j] = auxiliary[abs(index[j] - index[i])]; + double aux = auxiliary[abs(index[j] - index[i])]; + matrix[j * nb_errors + i] = aux; + matrix[im + j] = aux; } else { - matrix[j * nb_errors + i] = matrix[im + j] = 0; + matrix[j * nb_errors + i] = 0; + matrix[im + j] = 0; } } } diff --git a/libavfilter/framepool.c b/libavfilter/framepool.c index 20dbb342b6d..1d4a0908146 100644 --- a/libavfilter/framepool.c +++ b/libavfilter/framepool.c @@ -187,11 +187,14 @@ AVFrame *ff_frame_pool_get(FFFramePool *pool) } for (int i = 0; i < FFMIN(pool->planes, AV_NUM_DATA_POINTERS); i++) { + uint8_t *data; frame->buf[i] = av_buffer_pool_get(pool->pools[0]); if (!frame->buf[i]) goto fail; - frame->extended_data[i] = frame->data[i] = - (uint8_t *)FFALIGN((uintptr_t)frame->buf[i]->data, pool->align); + + data = (uint8_t *)FFALIGN((uintptr_t)frame->buf[i]->data, pool->align); + frame->extended_data[i] = data; + frame->data[i] = data; } for (int i = 0; i < frame->nb_extended_buf; i++) { frame->extended_buf[i] = av_buffer_pool_get(pool->pools[0]); diff --git a/libavfilter/vf_xbr.c b/libavfilter/vf_xbr.c index b2237749f53..fe0f029cedd 100644 --- a/libavfilter/vf_xbr.c +++ b/libavfilter/vf_xbr.c @@ -280,7 +280,7 @@ static av_always_inline void xbr_filter(const ThreadData *td, int jobnr, int nb_ const uint32_t I4 = sa3[pnext2]; if (n == 2) { - E[0] = E[1] = // 0, 1 + E[0] = E[1] = PE; // 0, 1 E[nl] = E[nl + 1] = PE; // 2, 3 FILT2(PE, PI, PH, PF, PG, PC, PD, PB, PA, G5, C4, G0, D0, C1, B1, F4, I4, H5, I5, A0, A1, 0, 1, nl, nl+1); @@ -288,8 +288,8 @@ static av_always_inline void xbr_filter(const ThreadData *td, int jobnr, int nb_ FILT2(PE, PA, PB, PD, PC, PG, PF, PH, PI, C1, G0, C4, F4, G5, H5, D0, A0, B1, A1, I4, I5, nl+1, nl, 1, 0); FILT2(PE, PG, PD, PH, PA, PI, PB, PF, PC, A0, I5, A1, B1, I4, F4, H5, G5, D0, G0, C1, C4, 1, nl+1, 0, nl); } else if (n == 3) { - E[0] = E[1] = E[2] = // 0, 1, 2 - E[nl] = E[nl+1] = E[nl+2] = // 3, 4, 5 + E[0] = E[1] = E[2] = PE; // 0, 1, 2 + E[nl] = E[nl+1] = E[nl+2] = PE; // 3, 4, 5 E[nl1] = E[nl1+1] = E[nl1+2] = PE; // 6, 7, 8 FILT3(PE, PI, PH, PF, PG, PC, PD, PB, PA, G5, C4, G0, D0, C1, B1, F4, I4, H5, I5, A0, A1, 0, 1, 2, nl, nl+1, nl+2, nl1, nl1+1, nl1+2); @@ -297,9 +297,9 @@ static av_always_inline void xbr_filter(const ThreadData *td, int jobnr, int nb_ FILT3(PE, PA, PB, PD, PC, PG, PF, PH, PI, C1, G0, C4, F4, G5, H5, D0, A0, B1, A1, I4, I5, nl1+2, nl1+1, nl1, nl+2, nl+1, nl, 2, 1, 0); FILT3(PE, PG, PD, PH, PA, PI, PB, PF, PC, A0, I5, A1, B1, I4, F4, H5, G5, D0, G0, C1, C4, 2, nl+2, nl1+2, 1, nl+1, nl1+1, 0, nl, nl1); } else if (n == 4) { - E[0] = E[1] = E[2] = E[3] = // 0, 1, 2, 3 - E[nl] = E[nl+1] = E[nl+2] = E[nl+3] = // 4, 5, 6, 7 - E[nl1] = E[nl1+1] = E[nl1+2] = E[nl1+3] = // 8, 9, 10, 11 + E[0] = E[1] = E[2] = E[3] = PE; // 0, 1, 2, 3 + E[nl] = E[nl+1] = E[nl+2] = E[nl+3] = PE; // 4, 5, 6, 7 + E[nl1] = E[nl1+1] = E[nl1+2] = E[nl1+3] = PE; // 8, 9, 10, 11 E[nl2] = E[nl2+1] = E[nl2+2] = E[nl2+3] = PE; // 12, 13, 14, 15 FILT4(PE, PI, PH, PF, PG, PC, PD, PB, PA, G5, C4, G0, D0, C1, B1, F4, I4, H5, I5, A0, A1, nl2+3, nl2+2, nl1+3, 3, nl+3, nl1+2, nl2+1, nl2, nl1+1, nl+2, 2, 1, nl+1, nl1, nl, 0); diff --git a/libavformat/apngdec.c b/libavformat/apngdec.c index 87be195df5c..d71247b6960 100644 --- a/libavformat/apngdec.c +++ b/libavformat/apngdec.c @@ -77,6 +77,7 @@ static int apng_probe(const AVProbeData *p) GetByteContext gb; int state = 0; uint32_t len, tag; + unsigned int width, height; bytestream2_init(&gb, p->buf, p->buf_size); @@ -99,7 +100,10 @@ static int apng_probe(const AVProbeData *p) case MKTAG('I', 'H', 'D', 'R'): if (len != 13) return 0; - if (av_image_check_size(bytestream2_get_be32(&gb), bytestream2_get_be32(&gb), 0, NULL)) + + width = bytestream2_get_be32(&gb); + height = bytestream2_get_be32(&gb); + if (av_image_check_size(width, height, 0, NULL)) return 0; bytestream2_skip(&gb, 9); state++; diff --git a/libavformat/takdec.c b/libavformat/takdec.c index 04c98c295e3..19385e9ed8f 100644 --- a/libavformat/takdec.c +++ b/libavformat/takdec.c @@ -173,14 +173,17 @@ static int tak_read_header(AVFormatContext *s) st->codecpar->extradata_size = size - 3; buffer = NULL; } else if (type == TAK_METADATA_LAST_FRAME) { + uint64_t pos; + int size; if (size != 11) { ret = AVERROR_INVALIDDATA; goto end; } init_get_bits8(&gb, buffer, size - 3); + pos = get_bits64(&gb, TAK_LAST_FRAME_POS_BITS); + size = get_bits(&gb, TAK_LAST_FRAME_SIZE_BITS); tc->mlast_frame = 1; - tc->data_end = get_bits64(&gb, TAK_LAST_FRAME_POS_BITS) + - get_bits(&gb, TAK_LAST_FRAME_SIZE_BITS); + tc->data_end = pos + size; av_freep(&buffer); } else if (type == TAK_METADATA_ENCODER) { av_log(s, AV_LOG_VERBOSE, "encoder version: %0X\n", diff --git a/libavutil/frame.c b/libavutil/frame.c index 185adbef680..4b0f700d9c1 100644 --- a/libavutil/frame.c +++ b/libavutil/frame.c @@ -185,13 +185,16 @@ static int get_audio_buffer(AVFrame *frame, int align) size = frame->linesize[0] + (size_t)align; for (int i = 0; i < FFMIN(planes, AV_NUM_DATA_POINTERS); i++) { + uint8_t *data; frame->buf[i] = av_buffer_alloc(size); if (!frame->buf[i]) { av_frame_unref(frame); return AVERROR(ENOMEM); } - frame->extended_data[i] = frame->data[i] = - (uint8_t *)FFALIGN((uintptr_t)frame->buf[i]->data, align); + + data = (uint8_t *)FFALIGN((uintptr_t)frame->buf[i]->data, align); + frame->extended_data[i] = data; + frame->data[i] = data; } for (int i = 0; i < planes - AV_NUM_DATA_POINTERS; i++) { frame->extended_buf[i] = av_buffer_alloc(size); -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
