[FFmpeg-devel] [PATCH 2/3] avformat/rtspdec: fix mem leaks on init fail

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Sun Oct 11 23:44:57 EEST 2020


Andriy Gelman:
> From: Andriy Gelman <andriy.gelman at gmail.com>
> 
> Fixes #6334
> 
> Signed-off-by: Andriy Gelman <andriy.gelman at gmail.com>
> ---
>  libavformat/rtspdec.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c
> index b519b6f1a2..623f478585 100644
> --- a/libavformat/rtspdec.c
> +++ b/libavformat/rtspdec.c
> @@ -720,7 +720,7 @@ static int rtsp_read_header(AVFormatContext *s)
>      if (rt->rtsp_flags & RTSP_FLAG_LISTEN) {
>          ret = rtsp_listen(s);
>          if (ret)
> -            return ret;
> +            goto fail;

This will add one ff_network_close() to this codepath. Is it really
certain that there was a corresponding ff_network_init()? (And where is
the ff_network_init() that is cancelled in rtsp_read_close() anyway?)

>      } else {
>          ret = ff_rtsp_connect(s);
>          if (ret)
> @@ -728,22 +728,25 @@ static int rtsp_read_header(AVFormatContext *s)
>  
>          rt->real_setup_cache = !s->nb_streams ? NULL :
>              av_mallocz_array(s->nb_streams, 2 * sizeof(*rt->real_setup_cache));
> -        if (!rt->real_setup_cache && s->nb_streams)
> -            return AVERROR(ENOMEM);
> +        if (!rt->real_setup_cache && s->nb_streams) {
> +            ret = AVERROR(ENOMEM);
> +            goto fail;
> +        }

With your patch, the calls to ff_network_init() and ff_network_close()
cancel each other out if the above allocation fails.

>          rt->real_setup = rt->real_setup_cache + s->nb_streams;
>  
>          if (rt->initial_pause) {
>              /* do not start immediately */
>          } else {
>              if ((ret = rtsp_read_play(s)) < 0) {
> -                ff_rtsp_close_streams(s);
> -                ff_rtsp_close_connections(s);
> -                return ret;
> +                goto fail;

This will add a TEARDOWN command to this and the above error path. Is
this something good?

>              }
>          }
>      }
> -
>      return 0;
> +
> +fail:
> +    rtsp_read_close(s);
> +    return ret;
>  }
>  
>  int ff_rtsp_tcp_read_packet(AVFormatContext *s, RTSPStream **prtsp_st,
> 



More information about the ffmpeg-devel mailing list