[FFmpeg-devel] [PATCH]Fix memleak if decoder init fails

[Michael Niedermayer]

On Thu, Mar 07, 2013 at 01:12:22PM +0100, Carl Eugen Hoyos wrote:
> On Thursday 07 March 2013 12:53:13 pm Michael Niedermayer wrote:
> > On Thu, Mar 07, 2013 at 10:38:42AM +0100, Carl Eugen Hoyos wrote:
> > > On Saturday 02 March 2013 10:02:39 pm Carl Eugen Hoyos wrote:
> > > > Both attached patches fix the memleak that happens if the decoder
> > > > initialisation fails, see ticket #1244 for an example.
> > >
> > > New patch attached that probably catches a few more cases.
> > >
> > > Please comment, Carl Eugen
> > >
> > >  ffmpeg.c |   10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > > 7d23fe5f1602bdde72583ef56923dbda109a2b70  patchtranscodeleak3.diff
> > > diff --git a/ffmpeg.c b/ffmpeg.c
> > > index 229a896..3f87cfc 100644
> > > --- a/ffmpeg.c
> > > +++ b/ffmpeg.c
> > > @@ -2542,6 +2542,16 @@ static int transcode_init(void)
> > >      }
> > >
> > >      if (ret) {
> > > +        for (i = 0; i < nb_output_streams; i++) {
> > > +            ost = output_streams[i];
> > > +            if (ost && ost->st && ost->st->codec)
> > > +                avcodec_close(ost->st->codec);
> > > +        }
> > > +        for (i = 0; i < nb_input_streams; i++) {
> > > +            ist = input_streams[i];
> > > +            if (ist && ist->st && ist->st->codec)
> > > +                avcodec_close(ist->st->codec);
> > > +        }
> >
> > This can probably call close on contexts that have been returned by
> > a failed open.
> 
> I suspect so, yes.
> 
> > Iam not sure this is safe ...
> 
> In this case I suggest attached original patch (again) that avoids exactly 
> this issue.
> 
> Carl Eugen

>  ffmpeg.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 510d48cf3e0f501aa3671cc32c36187fa75c0de9  patchtranscodeleak1.diff
> diff --git a/ffmpeg.c b/ffmpeg.c
> index c88665e..2ea3694 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -2444,8 +2444,13 @@ static int transcode_init(void)
>  
>      /* init input streams */
>      for (i = 0; i < nb_input_streams; i++)
> -        if ((ret = init_input_stream(i, error, sizeof(error))) < 0)
> +        if ((ret = init_input_stream(i, error, sizeof(error))) < 0) {
> +            for (i = 0; i < nb_output_streams; i++) {
> +                ost = output_streams[i];
> +                avcodec_close(ost->st->codec);
> +            }
>              goto dump_format;
> +        }
>  

can this affect dump_format ? that is does it access anything that
might be freed here ?

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130307/8738dacc/attachment.asc>