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

[Jun Zhao]

On 2017/6/7 11:17, Jun Zhao wrote:
> 
> 
> On 2017/6/7 9:22, Michael Niedermayer wrote:
>> 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
>>
> 
> Ok, will add {} for if.
> 
>>> +        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
>>
>> [...]
> 
> I think av_assert2(i <= (0xffffffff - 2)) have cover this condition, and maybe
> av_assert0(i <= (0xffffffff - 2)) is a better choice for this assert.
> 

I make a mistake for this comment, will clean the code logic to use put_bit64 when e >=16

>>
>>
>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>