[FFmpeg-devel] [PATCH v2] avformat/rtspdec: fix potential mem leak in listen mode

[Andriy Gelman]

On Fri, 27. Nov 09:25, Martin Storsjö wrote:
> On Thu, 26 Nov 2020, Andriy Gelman wrote:
> 
> > From: Andriy Gelman <andriy.gelman at gmail.com>
> > 
> > Currently a repeating setup request (with the same stream id) will
> > simply overwrite rtp_handle/transport_priv without freeing the
> > resources first. This is fixed by closing the previous setup request.
> > 
> > Signed-off-by: Andriy Gelman <andriy.gelman at gmail.com>
> > ---
> > libavformat/rtspdec.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> > 
> > diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c
> > index be11576913..221f44b20b 100644
> > --- a/libavformat/rtspdec.c
> > +++ b/libavformat/rtspdec.c
> > @@ -274,6 +274,17 @@ static int rtsp_read_setup(AVFormatContext *s, char* host, char *controlurl)
> >     rtsp_st   = rt->rtsp_streams[streamid];
> >     localport = rt->rtp_port_min;
> > 
> > +    /* check if the stream has already been setup */
> > +    if (rtsp_st->transport_priv) {
> > +        if (CONFIG_RTPDEC && rt->transport == RTSP_TRANSPORT_RDT)
> > +            ff_rdt_parse_close(rtsp_st->transport_priv);
> > +        else if (CONFIG_RTPDEC && rt->transport == RTSP_TRANSPORT_RTP)
> > +            ff_rtp_parse_close(rtsp_st->transport_priv);
> > +        rtsp_st->transport_priv = NULL;
> > +    }
> > +    if (rtsp_st->rtp_handle)
> > +        ffurl_closep(&rtsp_st->rtp_handle);
> > +
> >     if (request.transports[0].lower_transport == RTSP_LOWER_TRANSPORT_TCP) {
> 
> LGTM if tested for at least the RTP case. (RDT in listen mode is probably
> not supported at all, and I doubt you can find a client that would use that,
> unless manually crafting client requests to trigger it.)

Thanks, will apply both patches.

I tested the RTP case before.

For RDT, I used netcat to trigger this path. I modified the sdp so that RDT
transport is selected by adding
a=IsRealDataType:integer; 1

Then sent two setup requests with the same stream id. And a third setup request
with an invalid sequence number to error out. Valgrind didn't show any leaks.

--
Andriy