[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