[FFmpeg-devel] [Patch] fix ffprobe crash #3603
Stefano Sabatini
stefasab at gmail.com
Mon May 12 20:34:25 CEST 2014
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
> + 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
> + 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;
> + 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.]
--
FFmpeg = Fiendish & Frenzy Minimal Philosofic Embarassing Gangster
More information about the ffmpeg-devel
mailing list