[FFmpeg-devel] [Patch] fix ffprobe crash #3603

[Anshul]

On May 14, 2014 8:38:40 PM IST, Stefano Sabatini <stefasab at gmail.com> wrote:
>On date Tuesday 2014-05-13 09:55:45 +0530, Anshul encoded:
>[...]
>> From 53a415cec2b682a060ba1de154796dd32c0c81c9 Mon Sep 17 00:00:00
>2001
>> From: Anshul <er.anshul.maheshwari at gmail.com>
>> Date: Mon, 12 May 2014 23:15:20 +0530
>> Subject: [PATCH] ffprobe: fix crash because of new streams occuring
>> 
>> Fix ticket #3603
>> ---
>>  ffprobe.c | 23 ++++++++++++++++++-----
>>  1 file changed, 18 insertions(+), 5 deletions(-)
>> 
>> diff --git a/ffprobe.c b/ffprobe.c
>> index c6e0469..0090783 100644
>> --- a/ffprobe.c
>> +++ b/ffprobe.c
>> @@ -191,6 +191,7 @@ static const char unit_hertz_str[]          =
>"Hz"   ;
>>  static const char unit_byte_str[]           = "byte" ;
>>  static const char unit_bit_per_second_str[] = "bit/s";
>>  
>> +static int nb_streams;
>>  static uint64_t *nb_streams_packets;
>>  static uint64_t *nb_streams_frames;
>>  static int *selected_streams;
>> @@ -1632,6 +1633,13 @@ static void writer_register_all(void)
>>  #define print_section_header(s) writer_print_section_header(w, s)
>>  #define print_section_footer(s) writer_print_section_footer(w, s)
>>  
>> +#define CHECK_END if (ret < 0) goto end
>> +#define REALLOCZ_ARRAY_STREAM (ptr, cur_n, new_n)                   
>  \
>> +{                                                                   
>  \
>> +    ret = av_reallocp_array(&(ptr), (new_n), sizeof(*(ptr)));       
>  \
>> +    CHECK_END;                                                      
>  \
>
>> +    memset( (ptr) + (cur_n), 0, (new_n) - (cur_n) * sizeof(*(ptr))
>);  \
>
>Ehm, this should be ((new_n) - (cur_n)) * sizeof(*(ptr))
>
Done.
>> +}
>>  static inline int show_tags(WriterContext *w, AVDictionary *tags,
>int section_id)
>>  {
>>      AVDictionaryEntry *tag = NULL;
>> @@ -1893,6 +1901,12 @@ static int read_interval_packets(WriterContext
>*w, AVFormatContext *fmt_ctx,
>>          goto end;
>>      }
>>      while (!av_read_frame(fmt_ctx, &pkt)) {
>> +        if (fmt_ctx->nb_streams > nb_streams) {
>> +            REALLOCZ_ARRAY_STREAM(nb_streams_frames, nb_streams,
>fmt_ctx->nb_streams);
>> +           
>REALLOCZ_ARRAY_STREAM(nb_streams_packets,nb_streams,fmt_ctx->nb_streams);
>> +           
>REALLOCZ_ARRAY_STREAM(selected_streams,nb_streams,fmt_ctx->nb_streams);
>> +            nb_streams = fmt_ctx->nb_streams;
>> +        }
>>          if (selected_streams[pkt.stream_index]) {
>>              AVRational tb =
>fmt_ctx->streams[pkt.stream_index]->time_base;
>>  
>> @@ -2372,11 +2386,10 @@ static int probe_file(WriterContext *wctx,
>const char *filename)
>>      if (ret < 0)
>>          return ret;
>
>>  
>> -#define CHECK_END if (ret < 0) goto end
>> -
>> -    nb_streams_frames  = av_calloc(fmt_ctx->nb_streams,
>sizeof(*nb_streams_frames));
>> -    nb_streams_packets = av_calloc(fmt_ctx->nb_streams,
>sizeof(*nb_streams_packets));
>> -    selected_streams   = av_calloc(fmt_ctx->nb_streams,
>sizeof(*selected_streams));
>> +    nb_streams = fmt_ctx->nb_streams;
>> +    REALLOCZ_ARRAY_STREAM(nb_streams_frames,0,fmt_ctx->nb_streams);
>> +    REALLOCZ_ARRAY_STREAM(nb_streams_packets,0,fmt_ctx->nb_streams);
>> +    REALLOCZ_ARRAY_STREAM(selected_streams,0,fmt_ctx->nb_streams);
>
>What's the point of this hunk?
realloc is only safe if memory allocated by malloc, calloc or realloc. In ffmpeg av_calloc and av_malloc use mmap to allocate memory, so av_realloc is safe only if memory is allocated using av_realloc. It was disscussed with ubitux in start of this patch thread.

-Anshul



-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ffprobe-fix-crash-because-of-new-streams-occuring.patch
Type: application/octet-stream
Size: 2929 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140514/b32085b9/attachment.obj>