[PR] avcodec/error_resilience: fix null pointer dereferences and out-of-bounds access in error concealment (PR #24222)
yongdev via ffmpeg-devel <[email protected]>
| Newsgroups | gmane.comp.video.ffmpeg.devel |
|---|---|
| Message-ID | <[email protected]> |
PR #24222 opened by yongdev URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24222 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24222.patch In guess_mv() and related error concealment routines in libavcodec/error_resilience.c: 1. Pointers such as cur_pic.f, last_pic.f, cur_pic.motion_val[0], last_pic.motion_val[0], cur_pic.ref_index[0], er_temp_buffer, error_status_table, and decode_mb can be NULL or uninitialized when encountering corrupted or truncated bitstreams. 2. Boundary calculations for mb_xy in add_blocklist() and guess_mv() can access memory past allocated block/macroblock boundaries (max_blocks = mb_stride * mb_height). 3. Frame dimensions between cur_pic and last_pic may mismatch upon mid-stream dynamic resolution changes. Add NULL pointer guards, ensure frame dimensions match before motion vector estimation, and validate macroblock index bounds across add_blocklist() and reconstruction passes, preventing SIGSEGV crashes and heap buffer overflows during error resilience. Signed-off-by: yongdev <[email protected]> # Summary of changes Briefly describe what this PR does and why. <!-- If this PR requires new FATE test samples, attach them to the PR and list their target paths below (relative to the fate-suite root). Attached filenames must match the sample's filename: ```fate-samples # e.g. vorbis/new-sample.ogg ``` --> >From 1c5701a10b2b2e971f40c1dcb0b796d2253836d3 Mon Sep 17 00:00:00 2001 From: yongdev <[email protected]> Date: Thu, 20 Aug 2026 15:40:07 +0000 Subject: [PATCH] avcodec/error_resilience: fix null pointer dereferences and out-of-bounds access in error concealment In guess_mv() and related error concealment routines in libavcodec/error_resilience.c: 1. Pointers such as cur_pic.f, last_pic.f, cur_pic.motion_val[0], last_pic.motion_val[0], cur_pic.ref_index[0], er_temp_buffer, error_status_table, and decode_mb can be NULL or uninitialized when encountering corrupted or truncated bitstreams. 2. Boundary calculations for mb_xy in add_blocklist() and guess_mv() can access memory past allocated block/macroblock boundaries (max_blocks = mb_stride * mb_height). 3. Frame dimensions between cur_pic and last_pic may mismatch upon mid-stream dynamic resolution changes. Add NULL pointer guards, ensure frame dimensions match before motion vector estimation, and validate macroblock index bounds across add_blocklist() and reconstruction passes, preventing SIGSEGV crashes and heap buffer overflows during error resilience. Signed-off-by: yongdev <[email protected]> --- libavcodec/error_resilience.c | 102 +++++++++++++++++++++++----------- 1 file changed, 69 insertions(+), 33 deletions(-) diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c index 8cf5bc6a3c..777a60cf0c 100644 --- a/libavcodec/error_resilience.c +++ b/libavcodec/error_resilience.c @@ -398,13 +398,15 @@ static void v_block_filter(ERContext *s, uint8_t *dst, int w, int h, #define MV_CHANGED 4 #define MV_UNCHANGED 2 #define MV_LISTED 1 -static av_always_inline void add_blocklist(int (*blocklist)[2], int *blocklist_length, uint8_t *fixed, int mb_x, int mb_y, int mb_xy) +static av_always_inline void add_blocklist(int (*blocklist)[2], int *blocklist_length, uint8_t *fixed, int mb_x, int mb_y, int mb_xy, int max_blocks) { - if (fixed[mb_xy]) + if (mb_xy < 0 || mb_xy >= max_blocks || fixed[mb_xy]) return; fixed[mb_xy] = MV_LISTED; - blocklist[ *blocklist_length ][0] = mb_x; - blocklist[(*blocklist_length)++][1] = mb_y; + if (*blocklist_length < max_blocks) { + blocklist[ *blocklist_length ][0] = mb_x; + blocklist[(*blocklist_length)++][1] = mb_y; + } } static void guess_mv(ERContext *s) @@ -418,9 +420,22 @@ static void guess_mv(ERContext *s) int mb_x, mb_y; ptrdiff_t mot_step, mot_stride; int blocklist_length, next_blocklist_length; + const int max_blocks = s->mb_stride * s->mb_height; - if (s->last_pic.f && s->last_pic.f->data[0]) - mb_height = FFMIN(mb_height, (s->last_pic.f->height+15)>>4); + if (!s->cur_pic.f || !s->cur_pic.f->data[0] || + !s->last_pic.f || !s->last_pic.f->data[0]) + return; + if (!s->cur_pic.motion_val[0] || !s->last_pic.motion_val[0]) + return; + if (!s->er_temp_buffer || !s->error_status_table || !s->mb_index2xy || !s->cur_pic.mb_type) + return; + if (s->mb_width <= 0 || s->mb_height <= 0 || s->mb_stride <= 0) + return; + if (s->last_pic.f->width != s->cur_pic.f->width || + s->last_pic.f->height != s->cur_pic.f->height) + return; + + mb_height = FFMIN(mb_height, (s->last_pic.f->height+15)>>4); if (s->next_pic.f && s->next_pic.f->data[0]) mb_height = FFMIN(mb_height, (s->next_pic.f->height+15)>>4); @@ -437,10 +452,15 @@ static void guess_mv(ERContext *s) else ff_thread_progress_await(s->last_pic.progress, mb_height - 1); } - for (i = 0; i < mb_width * mb_height; i++) { + for (i = 0; i < mb_width * mb_height && i < s->mb_num; i++) { const int mb_xy = s->mb_index2xy[i]; int f = 0; - int error = s->error_status_table[mb_xy]; + int error; + + if (mb_xy < 0 || mb_xy >= max_blocks) + continue; + + error = s->error_status_table[mb_xy]; if (IS_INTRA(s->cur_pic.mb_type[mb_xy])) f = MV_FROZEN; // intra // FIXME check @@ -453,10 +473,13 @@ static void guess_mv(ERContext *s) else if(s->last_pic.f->data[0] && s->last_pic.motion_val[0]){ const int mb_y= mb_xy / s->mb_stride; const int mb_x= mb_xy % s->mb_stride; - const int mot_index= (mb_x + mb_y*mot_stride) * mot_step; - s->cur_pic.motion_val[0][mot_index][0]= s->last_pic.motion_val[0][mot_index][0]; - s->cur_pic.motion_val[0][mot_index][1]= s->last_pic.motion_val[0][mot_index][1]; - s->cur_pic.ref_index[0][4*mb_xy] = s->last_pic.ref_index[0][4*mb_xy]; + if (mb_x < mb_width && mb_y < mb_height) { + const int mot_index= (mb_x + mb_y*mot_stride) * mot_step; + s->cur_pic.motion_val[0][mot_index][0]= s->last_pic.motion_val[0][mot_index][0]; + s->cur_pic.motion_val[0][mot_index][1]= s->last_pic.motion_val[0][mot_index][1]; + if (s->cur_pic.ref_index[0] && s->last_pic.ref_index[0]) + s->cur_pic.ref_index[0][4*mb_xy] = s->last_pic.ref_index[0][4*mb_xy]; + } } } @@ -467,6 +490,8 @@ static void guess_mv(ERContext *s) const int mb_xy = mb_x + mb_y * s->mb_stride; int mv_dir = (s->last_pic.f && s->last_pic.f->data[0]) ? MV_DIR_FORWARD : MV_DIR_BACKWARD; + if (mb_xy < 0 || mb_xy >= max_blocks) + continue; if (IS_INTRA(s->cur_pic.mb_type[mb_xy])) continue; if (!(s->error_status_table[mb_xy] & ER_MV_ERROR)) @@ -474,8 +499,9 @@ static void guess_mv(ERContext *s) s->mv[0][0][0] = 0; s->mv[0][0][1] = 0; - s->decode_mb(s->opaque, 0, mv_dir, MV_TYPE_16X16, &s->mv, - mb_x, mb_y, 0, 0); + if (s->decode_mb) + s->decode_mb(s->opaque, 0, mv_dir, MV_TYPE_16X16, &s->mv, + mb_x, mb_y, 0, 0); } } return; @@ -485,11 +511,11 @@ static void guess_mv(ERContext *s) for (mb_y = 0; mb_y < mb_height; mb_y++) { for (mb_x = 0; mb_x < mb_width; mb_x++) { const int mb_xy = mb_x + mb_y * mb_stride; - if (fixed[mb_xy] == MV_FROZEN) { - if (mb_x) add_blocklist(blocklist, &blocklist_length, fixed, mb_x - 1, mb_y, mb_xy - 1); - if (mb_y) add_blocklist(blocklist, &blocklist_length, fixed, mb_x, mb_y - 1, mb_xy - mb_stride); - if (mb_x+1 < mb_width) add_blocklist(blocklist, &blocklist_length, fixed, mb_x + 1, mb_y, mb_xy + 1); - if (mb_y+1 < mb_height) add_blocklist(blocklist, &blocklist_length, fixed, mb_x, mb_y + 1, mb_xy + mb_stride); + if (mb_xy < max_blocks && fixed[mb_xy] == MV_FROZEN) { + if (mb_x) add_blocklist(blocklist, &blocklist_length, fixed, mb_x - 1, mb_y, mb_xy - 1, max_blocks); + if (mb_y) add_blocklist(blocklist, &blocklist_length, fixed, mb_x, mb_y - 1, mb_xy - mb_stride, max_blocks); + if (mb_x+1 < mb_width) add_blocklist(blocklist, &blocklist_length, fixed, mb_x + 1, mb_y, mb_xy + 1, max_blocks); + if (mb_y+1 < mb_height) add_blocklist(blocklist, &blocklist_length, fixed, mb_x, mb_y + 1, mb_xy + mb_stride, max_blocks); } } } @@ -548,7 +574,7 @@ static void guess_mv(ERContext *s) mv_predictor[pred_count][1] = s->cur_pic.motion_val[0][mot_index - mot_step][1]; ref[pred_count] = - s->cur_pic.ref_index[0][4 * (mb_xy - 1)]; + s->cur_pic.ref_index[0] ? s->cur_pic.ref_index[0][4 * (mb_xy - 1)] : 0; pred_count++; } if (mb_x + 1 < mb_width && fixed[mb_xy + 1] > 1) { @@ -557,7 +583,7 @@ static void guess_mv(ERContext *s) mv_predictor[pred_count][1] = s->cur_pic.motion_val[0][mot_index + mot_step][1]; ref[pred_count] = - s->cur_pic.ref_index[0][4 * (mb_xy + 1)]; + s->cur_pic.ref_index[0] ? s->cur_pic.ref_index[0][4 * (mb_xy + 1)] : 0; pred_count++; } if (mb_y > 0 && fixed[mb_xy - mb_stride] > 1) { @@ -566,7 +592,7 @@ static void guess_mv(ERContext *s) mv_predictor[pred_count][1] = s->cur_pic.motion_val[0][mot_index - mot_stride * mot_step][1]; ref[pred_count] = - s->cur_pic.ref_index[0][4 * (mb_xy - s->mb_stride)]; + s->cur_pic.ref_index[0] ? s->cur_pic.ref_index[0][4 * (mb_xy - s->mb_stride)] : 0; pred_count++; } if (mb_y + 1<mb_height && fixed[mb_xy + mb_stride] > 1) { @@ -575,7 +601,7 @@ static void guess_mv(ERContext *s) mv_predictor[pred_count][1] = s->cur_pic.motion_val[0][mot_index + mot_stride * mot_step][1]; ref[pred_count] = - s->cur_pic.ref_index[0][4 * (mb_xy + s->mb_stride)]; + s->cur_pic.ref_index[0] ? s->cur_pic.ref_index[0][4 * (mb_xy + s->mb_stride)] : 0; pred_count++; } if (pred_count == 0) @@ -634,7 +660,7 @@ skip_mean_and_median: prev_x = s->cur_pic.motion_val[0][mot_index][0]; prev_y = s->cur_pic.motion_val[0][mot_index][1]; - prev_ref = s->cur_pic.ref_index[0][4 * mb_xy]; + prev_ref = s->cur_pic.ref_index[0] ? s->cur_pic.ref_index[0][4 * mb_xy] : 0; /* last MV */ mv_predictor[pred_count][0] = prev_x; @@ -659,8 +685,9 @@ skip_mean_and_median: if (ref[j] < 0) continue; - s->decode_mb(s->opaque, ref[j], MV_DIR_FORWARD, - MV_TYPE_16X16, &s->mv, mb_x, mb_y, 0, 0); + if (s->decode_mb) + s->decode_mb(s->opaque, ref[j], MV_DIR_FORWARD, + MV_TYPE_16X16, &s->mv, mb_x, mb_y, 0, 0); if (mb_x > 0 && fixed[mb_xy - 1] > 1) { int k; @@ -700,8 +727,9 @@ skip_mean_and_median: s->cur_pic.motion_val[0][mot_index + i + j * mot_stride][1] = s->mv[0][0][1]; } - s->decode_mb(s->opaque, ref[best_pred], MV_DIR_FORWARD, - MV_TYPE_16X16, &s->mv, mb_x, mb_y, 0, 0); + if (s->decode_mb && ref[best_pred] >= 0) + s->decode_mb(s->opaque, ref[best_pred], MV_DIR_FORWARD, + MV_TYPE_16X16, &s->mv, mb_x, mb_y, 0, 0); if (s->mv[0][0][0] != prev_x || s->mv[0][0][1] != prev_y) { @@ -722,16 +750,16 @@ skip_mean_and_median: const int mb_y = blocklist[blocklist_index][1]; const int mb_xy = mb_x + mb_y * mb_stride; - if (fixed[mb_xy] & (MV_CHANGED|MV_UNCHANGED|MV_FROZEN)) { + if (mb_xy < max_blocks && (fixed[mb_xy] & (MV_CHANGED|MV_UNCHANGED|MV_FROZEN))) { fixed[mb_xy] = MV_FROZEN; if (mb_x > 0) - add_blocklist(next_blocklist, &next_blocklist_length, fixed, mb_x - 1, mb_y, mb_xy - 1); + add_blocklist(next_blocklist, &next_blocklist_length, fixed, mb_x - 1, mb_y, mb_xy - 1, max_blocks); if (mb_y > 0) - add_blocklist(next_blocklist, &next_blocklist_length, fixed, mb_x, mb_y - 1, mb_xy - mb_stride); + add_blocklist(next_blocklist, &next_blocklist_length, fixed, mb_x, mb_y - 1, mb_xy - mb_stride, max_blocks); if (mb_x + 1 < mb_width) - add_blocklist(next_blocklist, &next_blocklist_length, fixed, mb_x + 1, mb_y, mb_xy + 1); + add_blocklist(next_blocklist, &next_blocklist_length, fixed, mb_x + 1, mb_y, mb_xy + 1, max_blocks); if (mb_y + 1 < mb_height) - add_blocklist(next_blocklist, &next_blocklist_length, fixed, mb_x, mb_y + 1, mb_xy + mb_stride); + add_blocklist(next_blocklist, &next_blocklist_length, fixed, mb_x, mb_y + 1, mb_xy + mb_stride, max_blocks); } } av_assert0(next_blocklist_length <= mb_height * mb_width); @@ -1169,6 +1197,8 @@ void ff_er_frame_end(ERContext *s, int *decode_error_flags) continue; // inter with damaged MV if (!(error & ER_AC_ERROR)) continue; // undamaged inter + if (!s->cur_pic.motion_val[dir]) + continue; if (IS_8X8(mb_type)) { int mb_index = mb_x * 2 + mb_y * 2 * s->b8_stride; @@ -1253,6 +1283,9 @@ void ff_er_frame_end(ERContext *s, int *decode_error_flags) // if (error & ER_MV_ERROR) // continue; // inter data damaged FIXME is this good? + if (!s->cur_pic.f || !s->cur_pic.f->data[0] || !s->cur_pic.f->data[1] || !s->cur_pic.f->data[2]) + continue; + dest_y = s->cur_pic.f->data[0] + mb_x * 16 + mb_y * 16 * linesize[0]; dest_cb = s->cur_pic.f->data[1] + mb_x * 8 + mb_y * 8 * linesize[1]; dest_cr = s->cur_pic.f->data[2] + mb_x * 8 + mb_y * 8 * linesize[2]; @@ -1309,6 +1342,9 @@ void ff_er_frame_end(ERContext *s, int *decode_error_flags) if (!(error & ER_AC_ERROR)) continue; // undamaged + if (!s->cur_pic.f || !s->cur_pic.f->data[0] || !s->cur_pic.f->data[1] || !s->cur_pic.f->data[2]) + continue; + dest_y = s->cur_pic.f->data[0] + mb_x * 16 + mb_y * 16 * linesize[0]; dest_cb = s->cur_pic.f->data[1] + mb_x * 8 + mb_y * 8 * linesize[1]; dest_cr = s->cur_pic.f->data[2] + mb_x * 8 + mb_y * 8 * linesize[2]; -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]