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

Anshul anshul.ffmpeg at gmail.com
Tue May 13 06:25:45 CEST 2014


On May 13, 2014 5:23:07 AM IST, Stefano Sabatini <stefasab at gmail.com> wrote:
>In data Tuesday 2014-05-13 02:11:14 +0530, Anshul ha scritto:
>> On May 13, 2014 12:04:25 AM IST, Stefano Sabatini
><stefasab at gmail.com> wrote:
>> >On date Saturday 2014-05-10 13:44:40 +0530, Anshul encoded:
>> >> On May 10, 2014 3:45:41 AM IST, Michael Niedermayer
>> ><michaelni at gmx.at> wrote:
>> >> >On Fri, May 09, 2014 at 04:15:36PM +0530, anshul wrote:
>> >> >> On 05/09/2014 12:47 PM, Clément Boesch wrote:
>> >> >> >On Fri, May 09, 2014 at 09:15:53AM +0200, Clément Boesch
>wrote:
>> >> >> >>On Fri, May 09, 2014 at 12:33:36PM +0530, anshul wrote:
>> >> >> >>>On 05/09/2014 06:15 AM, Michael Niedermayer wrote:
>> >> >> >>>>>this patch consider all three things.
>> >> >> >>>>did you intend to attach anoter patch ?
>> >> >> >>>>iam asking as there was no patch attached to your last mail
>> >> >> >>>>
>> >> >> >>>>
>> >> >> >>>yes, I am sorry for that.
>> >> >> >>>
>> >> >> >>>-Anshul
>> >> >> >>> From 3ee1e369b42f0baa29706739f0b328615d20ebee Mon Sep 17
>> >00:00:00
>> >> >2001
>> >> >> >>>From: Anshul Maheshwari <er.anshul.maheshwari at gmail.com>
>> >> >> >>>Date: Thu, 8 May 2014 12:23:26 +0530
>> >> >> >>>Subject: [PATCH] ffprobe: fix crash because of new streams
>> >> >occuring
>> >> >> >>>
>> >> >> >>>Fix ticket #3603
>> >> >> >>>---
>> >> >> >>>  ffprobe.c | 19 ++++++++++++++-----
>> >> >> >>>  1 file changed, 14 insertions(+), 5 deletions(-)
>> >> >> >>>
>> >> >> >>>diff --git a/ffprobe.c b/ffprobe.c
>> >> >> >>>index c6e0469..5d6bf01 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;
>> >> >> >>>@@ -1893,6 +1894,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) {
>> >> >> >>>+            nb_streams_frames  =
>> >> >av_realloc(nb_streams_frames,fmt_ctx->nb_streams*
>> >> >sizeof(*nb_streams_frames));
>> >> >> >>>+            nb_streams_packets =
>> >> >av_realloc(nb_streams_packets,fmt_ctx->nb_streams*
>> >> >sizeof(*nb_streams_packets));
>> >> >> >>>+            selected_streams   =
>> >> >av_realloc(selected_streams,fmt_ctx->nb_streams*
>> >> >sizeof(*selected_streams));
>> >> >> >>space after ,
>> >> >> >>space before *
>> >> >> >for the mult obviously
>> >> >> >
>> >> >> >And speaking of this, you should use av_realloc_array for the
>> >> >overflow
>> >> >> >check.
>> >> >> >
>> >> >> >>space before (
>> >> >> >>
>> >> >> >for the if
>> >> >> >
>> >> >> >[...]
>> >> >> >
>> >> >> >
>> >> >> >
>> >> >> >_______________________________________________
>> >> >> >ffmpeg-devel mailing list
>> >> >> >ffmpeg-devel at ffmpeg.org
>> >> >> >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >> >> Please ignore previous patch, i don't know what is wrong with
>me.
>> >> >> Again attached new patch for fixing this crash
>> >> >> -Anshul
>> >> >
>> >> >>  ffprobe.c |   40 ++++++++++++++++++++++++++++++++++------
>> >> >>  1 file changed, 34 insertions(+), 6 deletions(-)
>> >> >> cefae455261a61fba6796d0dc5d261349ee44385 
>> >> >0001-ffprobe-fix-crash-because-of-new-streams-occuring.patch
>> >> >> From 12685c54a34b6ab5fcbc70cf86c4248dede61bdc Mon Sep 17
>00:00:00
>> >> >2001
>> >> >> From: Anshul Maheshwari <er.anshul.maheshwari at gmail.com>
>> >> >> Date: Fri, 9 May 2014 16:12:28 +0530
>> >> >> Subject: [PATCH] ffprobe: fix crash because of new streams
>> >occuring
>> >> >> 
>> >> >> Fix ticket #3603
>> >> >> ---
>> >> >>  ffprobe.c | 40 ++++++++++++++++++++++++++++++++++------
>> >> >>  1 file changed, 34 insertions(+), 6 deletions(-)
>> >> >> 
>> >> >> diff --git a/ffprobe.c b/ffprobe.c
>> >> >> index c6e0469..b9528e5 100644
>> >> >> --- a/ffprobe.c
>> >> >> +++ b/ffprobe.c
>> >> >> @@ -191,9 +191,10 @@ 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;
>> >> >> +static uint64_t *nb_streams_packets = NULL;
>> >> >> +static uint64_t *nb_streams_frames = NULL;
>> >> >> +static int *selected_streams = NULL;
>> >> >
>> >> >thats unrelated
>> >> >statics are already initialized to 0 by default
>> >> >
>> >> >
>> >> 
>> >> Done 
>> >> >>  
>> >> >>  static void ffprobe_cleanup(int ret)
>> >> >>  {
>> >> >> @@ -1893,6 +1894,25 @@ 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) {
>> >> >> +            ret = av_reallocp_array(&nb_streams_frames,
>> >> >fmt_ctx->nb_streams, sizeof(*nb_streams_frames));
>> >> >> +            if(ret)
>> >> >> +                goto end;
>> >> >> +            ret = av_reallocp_array(&nb_streams_packets,
>> >> >fmt_ctx->nb_streams, sizeof(*nb_streams_packets));
>> >> >> +            if(ret)
>> >> >> +                goto end;
>> >> >> +            ret = av_reallocp_array(&selected_streams,
>> >> >fmt_ctx->nb_streams, sizeof(*selected_streams));
>> >> >> +            if(ret)
>> >> >> +                goto end;
>> >> >> +            memset(nb_streams_frames + nb_streams, 0,
>> >> >> +                  (fmt_ctx->nb_streams - nb_streams) *
>> >> >sizeof(*nb_streams_frames));
>> >> >> +            memset(nb_streams_packets + nb_streams, 0,
>> >> >> +                  (fmt_ctx->nb_streams - nb_streams) *
>> >> >sizeof(*nb_streams_packets));
>> >> >> +            memset(selected_streams + nb_streams, 0,
>> >> >> +                  (fmt_ctx->nb_streams - nb_streams) *
>> >> >sizeof(*selected_streams));
>> >> >> +            nb_streams = fmt_ctx->nb_streams;
>> >> >> +        }
>> >> >
>> >> >> +#if 0
>> >> >>          if (selected_streams[pkt.stream_index]) {
>> >> >>              AVRational tb =
>> >> >fmt_ctx->streams[pkt.stream_index]->time_base;
>> >> >>  
>> >> >> @@ -1928,6 +1948,7 @@ static int
>> >read_interval_packets(WriterContext
>> >> >*w, AVFormatContext *fmt_ctx,
>> >> >>              }
>> >> >>          }
>> >> >>          av_free_packet(&pkt);
>> >> >> +#endif
>> >> >
>> >> >why is this code left in there and disabled ?
>> >> >
>> >> >[...]
>> >> 
>> >> Sorry for that I was debugging by ifdefs and left one there only,
>> >thats not required.
>> >> 
>> >> -Anshul
>> >> 
>> >> -- 
>> >> Sent from my Android device with K-9 Mail. Please excuse my
>brevity.
>> >
>> >> From 59c61b155ce290b80da4c50db84d0ecd6a139180 Mon Sep 17 00:00:00
>> >2001
>> >> From: Anshul <er.anshul.maheshwari at gmail.com>
>> >> Date: Sat, 10 May 2014 13:23:11 +0530
>> >> Subject: [PATCH] ffprobe: fix crash because of new streams
>occuring
>> >> 
>> >> Fix ticket #3603
>> >> ---
>> >>  ffprobe.c | 32 +++++++++++++++++++++++++++++---
>> >>  1 file changed, 29 insertions(+), 3 deletions(-)
>> >> 
>> >> diff --git a/ffprobe.c b/ffprobe.c
>> >> index c6e0469..b3d7aed 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;
>> >> @@ -1893,6 +1894,24 @@ 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) {
>> >> +            ret = av_reallocp_array(&nb_streams_frames,
>> >fmt_ctx->nb_streams, sizeof(*nb_streams_frames));
>> >
>> >> +            if(ret)
>> >> +                goto end;
>> >
>> >if (ret < 0)
>> >
>> >here and below
>> 
>> Done
>> 
>> >
>> >> +            ret = av_reallocp_array(&nb_streams_packets,
>> >fmt_ctx->nb_streams, sizeof(*nb_streams_packets));
>> >> +            if(ret)
>> >> +                goto end;
>> >> +            ret = av_reallocp_array(&selected_streams,
>> >fmt_ctx->nb_streams, sizeof(*selected_streams));
>> >> +            if(ret)
>> >> +                goto end;
>> >> +            memset(nb_streams_frames + nb_streams, 0,
>> >> +                  (fmt_ctx->nb_streams - nb_streams) *
>> >sizeof(*nb_streams_frames));
>> >> +            memset(nb_streams_packets + nb_streams, 0,
>> >> +                  (fmt_ctx->nb_streams - nb_streams) *
>> >sizeof(*nb_streams_packets));
>> >> +            memset(selected_streams + nb_streams, 0,
>> >> +                  (fmt_ctx->nb_streams - nb_streams) *
>> >sizeof(*selected_streams));
>> >
>> >nit: you can probably factorize this with a macro
>> >
>> I have tried to factorize. 
>> >> +            nb_streams = fmt_ctx->nb_streams;
>> >> +        }
>> >>          if (selected_streams[pkt.stream_index]) {
>> >>              AVRational tb =
>> >fmt_ctx->streams[pkt.stream_index]->time_base;
>> >>  
>> >> @@ -2373,10 +2392,17 @@ static int probe_file(WriterContext *wctx,
>> >const char *filename)
>> >>          return ret;
>> >>  
>> >>  #define CHECK_END if (ret < 0) goto end
>> >> +    nb_streams = fmt_ctx->nb_streams;
>> >> +    if(av_reallocp_array(&nb_streams_frames, fmt_ctx->nb_streams,
>> >sizeof(*nb_streams_frames)))
>> >> +        CHECK_END;
>> >
>> >are you missing the ret assignment?
>> >ret = av_reallocp_array(...);
>> >CHECK_END;
>> >
>> Yes i was.
>> 
>> >> +    if(av_reallocp_array(&nb_streams_packets,
>fmt_ctx->nb_streams,
>> >sizeof(*nb_streams_packets)))
>> >> +        CHECK_END;
>> >> +    if(av_reallocp_array(&selected_streams, fmt_ctx->nb_streams,
>> >sizeof(*selected_streams)))
>> >> +        CHECK_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));
>> >> +    memset(nb_streams_frames, 0, nb_streams *
>> >sizeof(*nb_streams_frames));
>> >> +    memset(nb_streams_packets, 0, nb_streams *
>> >sizeof(*nb_streams_packets));
>> >> +    memset(selected_streams, 0, nb_streams *
>> >sizeof(*selected_streams));
>> >
>> >[Will probably fix it and push it myself later if I'm still alive at
>> >the end of the day.]
>> 
>> I was already dead, so could not pull the git, and download that
>sample to test this latest one. 
>> 
>> -Anshul
>> 
>> -- 
>> Sent from my Android device with K-9 Mail. Please excuse my brevity.
>
>> From f0c899cae1568795fd5391d87bb1c8aef8328911 Mon Sep 17 00:00:00
>2001
>> From: Anshul <er.anshul.maheshwari at gmail.com>
>> Date: Tue, 13 May 2014 01:54:09 +0530
>> Subject: [PATCH] ffprobe: fix crash because of new streams occuring
>> 
>> Fix ticket #3603
>> ---
>>  ffprobe.c | 27 ++++++++++++++++++++++++---
>>  1 file changed, 24 insertions(+), 3 deletions(-)
>> 
>> diff --git a/ffprobe.c b/ffprobe.c
>> index c6e0469..22a9886 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;
>> @@ -1893,6 +1894,19 @@ 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) {
>> +#define REALLOCZ_ARRAY_STREAM (ptr, cur_n, new_n)                   
>  \
>> +{                                                                   
>  \
>> +    ret = av_reallocp_array(&(ptr), (new_n), sizeof(*(ptr)));       
>  \
>
>> +    if( ret < 0)                                                    
>  \
>> +        goto end;                                                   
>  \
>
>Nit: if_(ret < 0)
>
>> +    memset( (ptr) + (cur_n), 0, (new_n) - (cur_n) * sizeof(*(ptr)) )
>  \
>> +}
>
>> +            REALLOCZ_ARRAY_STREAM(nb_streams_frames, nb_streams,
>fmt_ctx->nb_streams);
>> +           
>REALLOCZ_ARRAY_STREAM(nb_streams_packets,fmt_ctx->nb_streams);
>> +           
>REALLOCZ_ARRAY_STREAM(selected_streams,fmt_ctx->nb_streams);
>
>inconsistent number of arguments
>
>> +            nb_streams = fmt_ctx->nb_streams;
>> +        }
>>          if (selected_streams[pkt.stream_index]) {
>>              AVRational tb =
>fmt_ctx->streams[pkt.stream_index]->time_base;
>>  
>> @@ -2373,10 +2387,17 @@ static int probe_file(WriterContext *wctx,
>const char *filename)
>>          return ret;
>>  
>>  #define CHECK_END if (ret < 0) goto end
>> +    nb_streams = fmt_ctx->nb_streams;
>> +    ret = av_reallocp_array(&nb_streams_frames, fmt_ctx->nb_streams,
>sizeof(*nb_streams_frames));
>> +        CHECK_END;
>> +    ret = av_reallocp_array(&nb_streams_packets,
>fmt_ctx->nb_streams, sizeof(*nb_streams_packets));
>> +        CHECK_END;
>> +    ret = av_reallocp_array(&selected_streams, fmt_ctx->nb_streams,
>sizeof(*selected_streams));
>> +        CHECK_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));
>> +    memset(nb_streams_frames, 0, nb_streams *
>sizeof(*nb_streams_frames));
>> +    memset(nb_streams_packets, 0, nb_streams *
>sizeof(*nb_streams_packets));
>> +    memset(selected_streams, 0, nb_streams *
>sizeof(*selected_streams));
>
>what about using the same macro as above?

New patch attached.
-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: 2926 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140513/ec1637d3/attachment.obj>


More information about the ffmpeg-devel mailing list