[FFmpeg-devel] [PATCH v2 1/3] avformat/sapenc: fix leak in sap_write_header()

[Lidong Yan]

Michael Niedermayer <michael at niedermayer.cc> write:

> On Thu, Jul 10, 2025 at 10:20:45AM +0800, Lidong Yan wrote:
> > In sap_write_header(), ff_format_set_url() assign new allocated new_url
> > to contexts[i]->url but forgot to free it later. Add for loop to free
> > contexts[i]->url before av_free(context).
> >
> > To prevent from writing free-for-loop in every return point, replace
> > `return 0` with `ret = 0` so normal execution can fall through fail
> > code.
> >
> > Signed-off-by: Lidong Yan <502024330056 at smail.nju.edu.cn>
> > ---
> >  libavformat/sapenc.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavformat/sapenc.c b/libavformat/sapenc.c
> > index 87a834a8d8..0567a754e2 100644
> > --- a/libavformat/sapenc.c
> > +++ b/libavformat/sapenc.c
> > @@ -233,7 +233,6 @@ static int sap_write_header(AVFormatContext *s)
> >          ret = AVERROR_INVALIDDATA;
> >          goto fail;
> >      }
> > -    av_freep(&contexts);
> >      av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", &sap->ann[pos]);
> >      pos += strlen(&sap->ann[pos]);
> >      sap->ann_size = pos;
> > @@ -244,11 +243,17 @@ static int sap_write_header(AVFormatContext *s)
> >          goto fail;
> >      }
> >
> > -    return 0;
> > +    ret = 0;
> >
> >  fail:
> > -    av_free(contexts);
> > -    sap_write_close(s);
> > +    if (contexts) {
> > +        for (i = 0; i < s->nb_streams; i++)
> > +            if (contexts[i])
> > +                av_free(contexts[i]->url);
> > +        av_free(contexts);
> > +    }
>
> contexts is an array of AVFormatContext, this does not look right
> also freeing the url of the AVFormatContexts that have been stored
> in priv_data a few lines earlier looks wrong
>

I am not sure how to fix then. Maybe I should leave this code alone.

Thanks,
Lidong