[FFmpeg-devel] [PATCH V2] lavc/golomb: Fix UE golomb overwrite issue.

[Michael Niedermayer]

On Mon, Jun 05, 2017 at 08:43:35AM +0800, Jun Zhao wrote:
> V2: Add Add set_ue_golomb_long() to support 32bits UE golomb and update the unit test.

>  golomb.h       |   20 +++++++++++++++++++-
>  put_bits.h     |   35 +++++++++++++++++++++++++++++++++++
>  tests/golomb.c |   19 +++++++++++++++++++
>  3 files changed, 73 insertions(+), 1 deletion(-)
> 491565dd491fc4ebd1717069d9c7655bfe0bd08a  0001-lavc-golomb-Fix-UE-golomb-overwrite-issue.patch
> From 6fe36e4e2a41f70e2a41c5eba90b5143b4eeba7b Mon Sep 17 00:00:00 2001
> From: Jun Zhao <jun.zhao at intel.com>
> Date: Fri, 2 Jun 2017 15:05:49 +0800
> Subject: [PATCH V2] lavc/golomb: Fix UE golomb overwrite issue.
> 
> put_bits just support write up to 31 bits, when write 32 bit in
> put_bits, it's will overwrite the bit buffer, because the default
> assert level is 0, the av_assert2(n <= 31 && value < (1U << n))
> in put_bits can not be trigger runtime. Add set_ue_golomb_long()
> to support 32bits UE golomb.
> 
> Signed-off-by: Jun Zhao <jun.zhao at intel.com>
> ---
>  libavcodec/golomb.h       | 20 +++++++++++++++++++-
>  libavcodec/put_bits.h     | 35 +++++++++++++++++++++++++++++++++++
>  libavcodec/tests/golomb.c | 19 +++++++++++++++++++
>  3 files changed, 73 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> index 0833aff468..47ab884282 100644
> --- a/libavcodec/golomb.h
> +++ b/libavcodec/golomb.h
> @@ -458,7 +458,7 @@ static inline int get_te(GetBitContext *s, int r, char *file, const char *func,
>  #endif /* TRACE */
>  
>  /**
> - * write unsigned exp golomb code.
> + * write unsigned exp golomb code. 2^16-2 at most.
>   */
>  static inline void set_ue_golomb(PutBitContext *pb, int i)
>  {
> @@ -473,6 +473,24 @@ static inline void set_ue_golomb(PutBitContext *pb, int i)
>  }
>  
>  /**
> + * write unsigned exp golomb code. 2^32-2 at most.
> + */
> +static inline void set_ue_golomb_long(PutBitContext *pb, uint32_t i)
> +{
> +    av_assert2(i <= (0xffffffff - 2));
> +
> +    if (i < 256)
> +        put_bits(pb, ff_ue_golomb_len[i], i + 1);
> +    else {

Please add {} for if else so its if { } else


> +        int e = av_log2(i + 1);
> +        if (e < 16)
> +            put_bits(pb, 2 * e + 1, i + 1);
> +        else

> +            put_bits64(pb, 2 * e + 1, i + 1);

put_bits64 tests for <32 it tests for ==64 neither are possible
here. And this is a inline function so these impossible code pathes
might get duplicated many times

[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Take away the freedom of one citizen and you will be jailed, take away
the freedom of all citizens and you will be congratulated by your peers
in Parliament.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170607/2b949a12/attachment.sig>