Re: [FFmpeg-devel] [PATCH v2] avformat/nutdec: Add check for avformat_new_stream

[Jiasheng Jiang <jiasheng@iscas.ac.cn>]

Michael Niedermayer:
>> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
>> index 0a8a700acf..4cbccb20d9 100644
>> --- a/libavformat/nutdec.c
>> +++ b/libavformat/nutdec.c
>> @@ -220,6 +220,10 @@ static int decode_main_header(NUTContext *nut)
>>      }
>>  
>>      GET_V(nut->time_base_count, tmp > 0 && tmp < INT_MAX / sizeof(AVRational) && tmp < length/2);
>> +
>> +    if (nut->time_base_count > NUT_MAX_STREAMS)
>> +        return AVERROR_INVALIDDATA;
>
> the code already checks against length/2. If you want to add to that
> that should be done at the same level and
> such a change should explain why the existing check is insufficent as
> well as why the new is correct
> and it should be in a patch seperate from other changes
> also a file with NUT_MAX_STREAMS streams could use more timebases in principle
> timebases need a lot less space than streams so they could have a slightly
> higher limit

Thanks, I will remove the check in v3.

>> +
>>      nut->time_base = av_malloc_array(nut->time_base_count, sizeof(AVRational));
>>      if (!nut->time_base)
>>          return AVERROR(ENOMEM);
>> @@ -351,8 +355,13 @@ static int decode_main_header(NUTContext *nut)
>>          ret = AVERROR(ENOMEM);
>>          goto fail;
>>      }
>> -    for (i = 0; i < stream_count; i++)
>> -        avformat_new_stream(s, NULL);
>> +    for (i = 0; i < stream_count; i++) {
>> +        if (!avformat_new_stream(s, NULL)) {
>> +            av_free(nut->stream);
> 
> freeing something and not clearing the pointer is a bad idea in general

You are right.
I will change av_free to av_freep.

Jiang

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".