[libav-devel] [PATCH 02/14] cbs_h264: Fix overflow in shifts

Alexandra Hájková alexandra.khirnova at gmail.com
Mon May 13 10:38:59 CEST 2019


On Sat, May 4, 2019 at 2:14 PM Luca Barbato <lu_zero at gentoo.org> wrote:
>
> From: Mark Thompson <sw at jkqxz.net>
>
> 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 <andreas.rheinhardt at googlemail.com>
> ---
>  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 1aa7888584..92c1b67862 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 4c6f421d19..54265d8e0e 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;
> --
> 2.12.2
>

OK


More information about the libav-devel mailing list