Re: [libav-devel] [PATCH 3/4] drawtext: Set an explicit allocation limit

[Luca Barbato]

On 01/08/15 20:46, Anton Khirnov wrote:
> Quoting Luca Barbato (2015-08-01 11:22:12)
>> And make sure to not have dangling pointers.
>> ---
>>  libavfilter/vf_drawtext.c | 17 ++++++++++-------
>>  1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c
>> index 15c994f..62993cf 100644
>> --- a/libavfilter/vf_drawtext.c
>> +++ b/libavfilter/vf_drawtext.c
>> @@ -524,28 +524,31 @@ static inline int is_newline(uint32_t c)
>>  static int expand_strftime(DrawTextContext *s)
>>  {
>>      struct tm ltime;
>> -    time_t now   = time(0);
>> -    uint8_t *buf = s->expanded_text;
>> -    int buf_size = s->expanded_text_size;
>> +    time_t now       = time(0);
>> +    uint8_t *buf     = s->expanded_text;
>> +    int64_t buf_size = s->expanded_text_size;
> 
> Why signed?

I just needed something large enough, unsigned probably is a better idea.

> 
>> +    int ret;
>>  
>>      if (!buf)
>>          buf_size = 2 * strlen(s->text) + 1;
>>  
>>      localtime_r(&now, &ltime);
>>  
>> -    while ((buf = av_realloc(buf, buf_size))) {
>> +    while ((ret = av_reallocp(&buf, buf_size)) >= 0) {
>>          *buf = 1;
>>          if (strftime(buf, buf_size, s->text, &ltime) != 0 || *buf == 0)
>>              break;
>>          buf_size *= 2;
>> +        if (buf_size > SIZE_MAX) {
>> +            ret = AVERROR(EINVAL);
>> +            break;
>> +        }
> 
> I think it'd be better to a standard overflow check before modifying
> buf_size and not assume some relationships between size_t and int64.

Sure.


> When buf is NULL, buf_size is still a random number. Better to
> explicitly write zero on failure here.

Yeah it is a better idea.

lu
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel