[FFmpeg-devel] [Patch] fix ffprobe crash #3603
Stefano Sabatini
stefasab at gmail.com
Tue May 13 01:53:07 CEST 2014
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?
More information about the ffmpeg-devel
mailing list