From b0b0535178ae601bb1f61330501bfae208ab80f8 Mon Sep 17 00:00:00 2001 From: Jeff Jiang Date: Tue, 29 Sep 2026 13:20:55 -0400 Subject: [PATCH] rocDecode/Parser: Bound the AVC ref pic list modification and MMCO arrays. - ParseSliceHeader() filled ref_pic_list.modification_l0 / modification_l1 and dec_ref_pic_marking.mmco with do/while loops whose index was bounded only by an in-bitstream terminator, modification_of_pic_nums_idc equal to 3 and memory_management_control_operation equal to 0. All three arrays were sized AVC_MAX_REF_PICTURE_NUM, so a stream that simply never emits the terminator walked the index past 32 and wrote out of bounds. - Cap the modification loops at num_ref_idx_lX_active_minus1 + 1, the bound 7.4.3.1 places on the entries with modification_of_pic_nums_idc not equal to 3, allowing for the terminating entry. num_ref_idx_lX_active_minus1 is already range checked to 31 or less on every path that reaches these loops, so the largest index is 32. - AVC_MAX_REF_PICTURE_NUM was also one entry short of what those bounds permit, so a bare capacity check would have rejected conforming streams. Size the two modification arrays with AVC_MAX_REF_LIST_MOD_NUM, 32 entries plus the terminator. - The spec places no single limit on the MMCO count, so derive one: each of the up to 32 reference fields can be marked unused for reference by operation 1 or 2 and be assigned a long term frame index by operation 3 or 6, plus one operation 4, one operation 5, and the terminating operation 0. Apply that as AVC_MAX_MMCO_NUM and cap the loop with it. - ModifiyRefList() advanced through ref_pic_list.modification_l0 regardless of which list it was called for, so on the list 1 call it searched for the terminator in list 0's array. When ref_pic_list_modification_flag_l0 is 0 that array holds no terminator for the current slice and the walk ran off the end. Advance through the list the caller passed in. This also fixes list 1 being built from list 0's operations whenever an L1 modification has more than one entry. - PrintSliceHeader() dumped a hardcoded 32 entries of each array. Follow the new sizes and stop at the terminator, and at mmco_count for the MMCO dump. Co-Authored-By: Claude Opus 5 (1M context) --- projects/rocdecode/src/parser/avc_defines.h | 13 ++++++++++--- projects/rocdecode/src/parser/avc_parser.cpp | 20 ++++++++++++++++---- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/projects/rocdecode/src/parser/avc_defines.h b/projects/rocdecode/src/parser/avc_defines.h index a395494633c9..6b8475d99559 100644 --- a/projects/rocdecode/src/parser/avc_defines.h +++ b/projects/rocdecode/src/parser/avc_defines.h @@ -33,6 +33,13 @@ THE SOFTWARE. #define AVC_MAX_REF_FRAME_NUM 16 #define AVC_MAX_REF_PICTURE_NUM 32 +// At most num_ref_idx_lX_active_minus1 + 1 (<= 32) modification entries, plus the terminating +// entry with modification_of_pic_nums_idc equal to 3. +#define AVC_MAX_REF_LIST_MOD_NUM (AVC_MAX_REF_PICTURE_NUM + 1) +// Each of the up to 32 reference fields can be marked unused for reference (operation 1 or 2) and +// be assigned a long term frame index (operation 3 or 6), plus one operation 4, one operation 5, +// and the terminating operation 0. +#define AVC_MAX_MMCO_NUM (2 * AVC_MAX_REF_PICTURE_NUM + 3) #define AVC_MAX_DPB_FRAMES 16 #define AVC_MAX_DPB_FIELDS AVC_MAX_DPB_FRAMES * 2 @@ -261,9 +268,9 @@ typedef struct { typedef struct { uint32_t ref_pic_list_modification_flag_l0; // u(1) - AvcListMod modification_l0[AVC_MAX_REF_PICTURE_NUM]; + AvcListMod modification_l0[AVC_MAX_REF_LIST_MOD_NUM]; uint32_t ref_pic_list_modification_flag_l1; // u(1) - AvcListMod modification_l1[AVC_MAX_REF_PICTURE_NUM]; + AvcListMod modification_l1[AVC_MAX_REF_LIST_MOD_NUM]; } AvcRefPicListMod; // Prediction weight table syntax. AVC spec. 7.3.3.2. @@ -301,7 +308,7 @@ typedef struct { uint32_t no_output_of_prior_pics_flag; // u(1) uint32_t long_term_reference_flag; // u(1) uint32_t adaptive_ref_pic_marking_mode_flag; // u(1) - AvcMmco mmco[AVC_MAX_REF_PICTURE_NUM]; + AvcMmco mmco[AVC_MAX_MMCO_NUM]; uint32_t mmco_count; } AvcDecRefPicMarking; diff --git a/projects/rocdecode/src/parser/avc_parser.cpp b/projects/rocdecode/src/parser/avc_parser.cpp index 5b10a680134c..31ea7d776020 100644 --- a/projects/rocdecode/src/parser/avc_parser.cpp +++ b/projects/rocdecode/src/parser/avc_parser.cpp @@ -1499,6 +1499,8 @@ ParserResult AvcVideoParser::ParseSliceHeader(uint8_t *p_stream, size_t stream_s if (p_slice_header->ref_pic_list.ref_pic_list_modification_flag_l0 == 1) { i = 0; do { + // The terminating entry can be at most at index num_ref_idx_l0_active_minus1 + 1. + CHECK_ALLOWED_MAX("ref_pic_list_modification_l0 entry count", i, static_cast(p_slice_header->num_ref_idx_l0_active_minus1) + 1); modification_of_pic_nums_idc = Parser::ExpGolomb::ReadUe(p_stream, offset); CHECK_ALLOWED_RANGE("modification_of_pic_nums_idc", modification_of_pic_nums_idc, 0, 3); p_slice_header->ref_pic_list.modification_l0[i].modification_of_pic_nums_idc = modification_of_pic_nums_idc; @@ -1518,6 +1520,8 @@ ParserResult AvcVideoParser::ParseSliceHeader(uint8_t *p_stream, size_t stream_s if (p_slice_header->ref_pic_list.ref_pic_list_modification_flag_l1 == 1) { i = 0; do { + // The terminating entry can be at most at index num_ref_idx_l1_active_minus1 + 1. + CHECK_ALLOWED_MAX("ref_pic_list_modification_l1 entry count", i, static_cast(p_slice_header->num_ref_idx_l1_active_minus1) + 1); modification_of_pic_nums_idc = Parser::ExpGolomb::ReadUe(p_stream, offset); CHECK_ALLOWED_RANGE("modification_of_pic_nums_idc", modification_of_pic_nums_idc, 0, 3); p_slice_header->ref_pic_list.modification_l1[i].modification_of_pic_nums_idc = modification_of_pic_nums_idc; @@ -1621,6 +1625,7 @@ ParserResult AvcVideoParser::ParseSliceHeader(uint8_t *p_stream, size_t stream_s if (p_slice_header->dec_ref_pic_marking.adaptive_ref_pic_marking_mode_flag == 1) { i = 0; do { + CHECK_ALLOWED_MAX("mmco entry count", i, AVC_MAX_MMCO_NUM - 1); memory_management_control_operation = Parser::ExpGolomb::ReadUe(p_stream, offset); CHECK_ALLOWED_RANGE("memory_management_control_operation", memory_management_control_operation, 0, 6); p_slice_header->dec_ref_pic_marking.mmco[i].memory_management_control_operation = memory_management_control_operation; @@ -2729,6 +2734,7 @@ ParserResult AvcVideoParser::ModifiyRefList(AvcPicture *ref_pic_list_x, AvcListM int num_short_term_pics = curr_pic_.pic_structure == kFrame ? dpb_buffer_.num_short_term : dpb_buffer_.num_short_term_ref_fields; int num_long_term_pics = curr_pic_.pic_structure == kFrame ? dpb_buffer_.num_long_term : dpb_buffer_.num_long_term_ref_fields; AvcPicture ref_pic_list_mod[AVC_MAX_REF_PICTURE_NUM + 1]; + AvcListMod *p_list_mod_base = p_list_mod; // modification_l0 or modification_l1 of the current slice int i, c_idx, n_idx; memcpy(ref_pic_list_mod, ref_pic_list_x, sizeof(AvcPicture) * num_ref_idx_lx_active); @@ -2812,7 +2818,7 @@ ParserResult AvcVideoParser::ModifiyRefList(AvcPicture *ref_pic_list_x, AvcListM } } } - p_list_mod = &p_slice_header->ref_pic_list.modification_l0[ref_idx_lx]; + p_list_mod = &p_list_mod_base[ref_idx_lx]; } memcpy(ref_pic_list_x, ref_pic_list_mod, sizeof(AvcPicture) * num_ref_idx_lx_active); @@ -3606,16 +3612,22 @@ void AvcVideoParser::PrintSliceHeader(AvcSliceHeader *p_slice_header) { MSG("ref_pic_list_modification_flag_l0 = " << p_slice_header->ref_pic_list.ref_pic_list_modification_flag_l0); if ( p_slice_header->ref_pic_list.ref_pic_list_modification_flag_l0 ){ MSG("Modification operations for list 0: "); - for (j = 0; j < AVC_MAX_REF_PICTURE_NUM; j++) { + for (j = 0; j < AVC_MAX_REF_LIST_MOD_NUM; j++) { MSG_NO_NEWLINE("(" << p_slice_header->ref_pic_list.modification_l0[j].modification_of_pic_nums_idc << ", " << p_slice_header->ref_pic_list.modification_l0[j].abs_diff_pic_num_minus1 << ", " << p_slice_header->ref_pic_list.modification_l0[j].long_term_pic_num << ") "); + if (p_slice_header->ref_pic_list.modification_l0[j].modification_of_pic_nums_idc == 3) { + break; + } } MSG(""); } MSG("ref_pic_list_modification_flag_l1 = " << p_slice_header->ref_pic_list.ref_pic_list_modification_flag_l1); if ( p_slice_header->ref_pic_list.ref_pic_list_modification_flag_l1 ) { MSG("Modification operations for list 1: "); - for (j = 0; j < AVC_MAX_REF_PICTURE_NUM; j++) { + for (j = 0; j < AVC_MAX_REF_LIST_MOD_NUM; j++) { MSG_NO_NEWLINE("(" << p_slice_header->ref_pic_list.modification_l1[j].modification_of_pic_nums_idc << ", " << p_slice_header->ref_pic_list.modification_l1[j].abs_diff_pic_num_minus1 << ", " << p_slice_header->ref_pic_list.modification_l1[j].long_term_pic_num << ") "); + if (p_slice_header->ref_pic_list.modification_l1[j].modification_of_pic_nums_idc == 3) { + break; + } } MSG(""); } @@ -3629,7 +3641,7 @@ void AvcVideoParser::PrintSliceHeader(AvcSliceHeader *p_slice_header) { MSG("adaptive_ref_pic_marking_mode_flag = " << refMarking->adaptive_ref_pic_marking_mode_flag); if ( refMarking->adaptive_ref_pic_marking_mode_flag ) { MSG("mmco_count = " << refMarking->mmco_count); - for (j = 0; j < AVC_MAX_REF_PICTURE_NUM; j++) { + for (j = 0; j < refMarking->mmco_count; j++) { MSG_NO_NEWLINE("(" << refMarking->mmco[j].memory_management_control_operation << ", " << refMarking->mmco[j].difference_of_pic_nums_minus1 << ", " << refMarking->mmco[j].long_term_pic_num << ", " << refMarking->mmco[j].long_term_frame_idx << ", " << refMarking->mmco[j].max_long_term_frame_idx_plus1 << ") "); } MSG("");