From 0e782661d63b39d729b2167e75a690b4e2934740 Mon Sep 17 00:00:00 2001 From: Mark Thompson Date: Sun, 11 Mar 2018 14:40:23 +0000 Subject: [PATCH] cbs_h264: Fix overflow in shifts The type of the result of a shift operation is unaffected by the type of the right operand, so some existing code overflows with undefined behaviour when the element length is 32. Add a helper macro to calculate the maximum value correctly and then use it everywhere this pattern appears. Found-by: Andreas Rheinhardt --- libavcodec/cbs_h264_syntax_template.c | 22 +++++++++++----------- libavcodec/cbs_internal.h | 4 ++++ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/libavcodec/cbs_h264_syntax_template.c b/libavcodec/cbs_h264_syntax_template.c index f58dee8a25..b5cd0b2310 100644 --- a/libavcodec/cbs_h264_syntax_template.c +++ b/libavcodec/cbs_h264_syntax_template.c @@ -342,8 +342,8 @@ static int FUNC(sps_extension)(CodedBitstreamContext *ctx, RWContext *rw, flag(alpha_incr_flag); bits = current->bit_depth_aux_minus8 + 9; - u(bits, alpha_opaque_value, 0, (1 << bits) - 1); - u(bits, alpha_transparent_value, 0, (1 << bits) - 1); + u(bits, alpha_opaque_value, 0, MAX_UINT_BITS(bits)); + u(bits, alpha_transparent_value, 0, MAX_UINT_BITS(bits)); } flag(additional_extension_flag); @@ -483,10 +483,10 @@ static int FUNC(sei_buffering_period)(CodedBitstreamContext *ctx, RWContext *rw, length = sps->vui.nal_hrd_parameters.initial_cpb_removal_delay_length_minus1 + 1; xu(length, initial_cpb_removal_delay[SchedSelIdx], current->nal.initial_cpb_removal_delay[i], - 0, (1 << (uint64_t)length) - 1); + 1, MAX_UINT_BITS(length)); xu(length, initial_cpb_removal_delay_offset[SchedSelIdx], current->nal.initial_cpb_removal_delay_offset[i], - 0, (1 << (uint64_t)length) - 1); + 0, MAX_UINT_BITS(length)); } } @@ -495,10 +495,10 @@ static int FUNC(sei_buffering_period)(CodedBitstreamContext *ctx, RWContext *rw, length = sps->vui.vcl_hrd_parameters.initial_cpb_removal_delay_length_minus1 + 1; xu(length, initial_cpb_removal_delay[SchedSelIdx], current->vcl.initial_cpb_removal_delay[i], - 0, (1 << (uint64_t)length) - 1); + 1, MAX_UINT_BITS(length)); xu(length, initial_cpb_removal_delay_offset[SchedSelIdx], current->vcl.initial_cpb_removal_delay_offset[i], - 0, (1 << (uint64_t)length) - 1); + 0, MAX_UINT_BITS(length)); } } @@ -548,7 +548,7 @@ static int FUNC(sei_pic_timestamp)(CodedBitstreamContext *ctx, RWContext *rw, if (time_offset_length > 0) u(time_offset_length, time_offset, - 0, (1 << (uint64_t)time_offset_length) - 1); + 0, MAX_UINT_BITS(time_offset_length)); else infer(time_offset, 0); @@ -600,9 +600,9 @@ static int FUNC(sei_pic_timing)(CodedBitstreamContext *ctx, RWContext *rw, } u(hrd->cpb_removal_delay_length_minus1 + 1, cpb_removal_delay, - 0, (1 << (uint64_t)hrd->cpb_removal_delay_length_minus1) + 1); + 0, MAX_UINT_BITS(hrd->cpb_removal_delay_length_minus1 + 1)); u(hrd->dpb_output_delay_length_minus1 + 1, dpb_output_delay, - 0, (1 << (uint64_t)hrd->dpb_output_delay_length_minus1) + 1); + 0, MAX_UINT_BITS(hrd->dpb_output_delay_length_minus1 + 1)); } if (sps->vui.pic_struct_present_flag) { @@ -1123,7 +1123,7 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw, u(2, colour_plane_id, 0, 2); u(sps->log2_max_frame_num_minus4 + 4, frame_num, - 0, (1 << (sps->log2_max_frame_num_minus4 + 4)) - 1); + 0, MAX_UINT_BITS(sps->log2_max_frame_num_minus4 + 4)); if (!sps->frame_mbs_only_flag) { flag(field_pic_flag); @@ -1141,7 +1141,7 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw, if (sps->pic_order_cnt_type == 0) { u(sps->log2_max_pic_order_cnt_lsb_minus4 + 4, pic_order_cnt_lsb, - 0, (1 << (sps->log2_max_pic_order_cnt_lsb_minus4 + 4)) - 1); + 0, MAX_UINT_BITS(sps->log2_max_pic_order_cnt_lsb_minus4 + 4)); if (pps->bottom_field_pic_order_in_frame_present_flag && !current->field_pic_flag) se(delta_pic_order_cnt_bottom, INT32_MIN + 1, INT32_MAX); diff --git a/libavcodec/cbs_internal.h b/libavcodec/cbs_internal.h index 5674803472..be540e2a44 100644 --- a/libavcodec/cbs_internal.h +++ b/libavcodec/cbs_internal.h @@ -79,6 +79,10 @@ int ff_cbs_write_unsigned(CodedBitstreamContext *ctx, PutBitContext *pbc, int width, const char *name, uint32_t value, uint32_t range_min, uint32_t range_max); +// The largest value representable in N bits, suitable for use as +// range_max in the above functions. +#define MAX_UINT_BITS(length) ((UINT64_C(1) << (length)) - 1) + extern const CodedBitstreamType ff_cbs_type_h264; extern const CodedBitstreamType ff_cbs_type_h265;