| Message ID | 20201127003721.68430-1-andriy.gelman@gmail.com |
|---|---|
| State | Accepted |
| Commit | f3891430fc15cc8a1b7a16916030962e0a4d609c |
| Headers | show |
| Series | [FFmpeg-devel,v2] avformat/rtspdec: fix potential mem leak in listen mode | expand |
| Context | Check | Description |
|---|---|---|
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
On Thu, 26 Nov 2020, Andriy Gelman wrote: > From: Andriy Gelman <andriy.gelman@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@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.) // Martin
On Fri, 27. Nov 09:25, Martin Storsjö wrote: > On Thu, 26 Nov 2020, Andriy Gelman wrote: > > > From: Andriy Gelman <andriy.gelman@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@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
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) { rt->lower_transport = RTSP_LOWER_TRANSPORT_TCP; if ((ret = ff_rtsp_open_transport_ctx(s, rtsp_st))) {