[FFmpeg-devel] [PATCH] avcodec/h2645_parse: zero initialize the rbsp buffer

[James Almer]

On 8/27/2019 4:23 AM, Andreas Rheinhardt wrote:
> James Almer:
>> Fixes ticket #8093
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>  libavcodec/h2645_parse.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
>> index 24658b3dfa..307e8643e6 100644
>> --- a/libavcodec/h2645_parse.c
>> +++ b/libavcodec/h2645_parse.c
>> @@ -345,13 +345,18 @@ static int find_next_start_code(const uint8_t *buf, const uint8_t *next_avc)
>>  
>>  static void alloc_rbsp_buffer(H2645RBSP *rbsp, unsigned int size, int use_ref)
>>  {
>> +    int min_size = size;
>> +
>>      if (size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE)
>>          goto fail;
>>      size += AV_INPUT_BUFFER_PADDING_SIZE;
>>  
>>      if (rbsp->rbsp_buffer_alloc_size >= size &&
>> -        (!rbsp->rbsp_buffer_ref || av_buffer_is_writable(rbsp->rbsp_buffer_ref)))
>> +        (!rbsp->rbsp_buffer_ref || av_buffer_is_writable(rbsp->rbsp_buffer_ref))) {
>> +        av_assert0(rbsp->rbsp_buffer);
>> +        memset(rbsp->rbsp_buffer + min_size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>>          return;
>> +    }
>>  
>>      size = FFMIN(size + size / 16 + 32, INT_MAX);
>>  
>> @@ -360,7 +365,7 @@ static void alloc_rbsp_buffer(H2645RBSP *rbsp, unsigned int size, int use_ref)
>>      else
>>          av_free(rbsp->rbsp_buffer);
>>  
>> -    rbsp->rbsp_buffer = av_malloc(size);
>> +    rbsp->rbsp_buffer = av_mallocz(size);
>>      if (!rbsp->rbsp_buffer)
>>          goto fail;
>>      rbsp->rbsp_buffer_alloc_size = size;
>>
> When I wrote 992532ee3122d7938a7581988eea401b57de8189, I thought that
> all the code working with the rbsp just needs padding, but not zeroed
> padding, because after all, the data after an RBSP might be the
> beginning of the next RBSP (or if it is the last NAL unit in the RBSP
> buffer, then only AV_INPUT_BUFFER_PADDING_SIZE bytes are guaranteed to
> be zero, the rest could be leftover stuff from earlier packets).
> Therefore I didn't keep this. I just thought of the users of these
> functions and not of analyzers like Valgrind, sorry.
> If I am not mistaken, then av_mallocz is enough for everything, but
> the patch looks good to me either way.

Pushed then, thanks.