[FFmpeg-devel,v2,1/8] avformat/utils: Fix memleaks

Submitted by Andreas Rheinhardt on Aug. 19, 2019, 9:56 p.m.

Details

Message ID 20190819215624.49795-1-andreas.rheinhardt@gmail.com
State New
Headers show

Commit Message

Andreas Rheinhardt Aug. 19, 2019, 9:56 p.m.
ff_read_packet had several potential memleaks:
1. If av_packet_make_refcounted fails, it means that the packet is not
refcounted, but it could nevertheless carry side data and therefore
needs to be unreferenced.
2. If a packet happens to have an illegal stream index (i.e. one that
does not correspond to a stream), it should nevertheless be
unreferenced.
3. If putting a packet on a packet list fails, it wasn't unreferenced.

Furthermore, read_frame_internal leaked a packet's (side) data if a
context update was required and failed.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
The two memleaks in read_frame_internal have been added in v2.

 libavformat/utils.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

Comments

Andreas Rheinhardt Sept. 16, 2019, 10:02 p.m.
Andreas Rheinhardt:
> ff_read_packet had several potential memleaks:
> 1. If av_packet_make_refcounted fails, it means that the packet is not
> refcounted, but it could nevertheless carry side data and therefore
> needs to be unreferenced.
> 2. If a packet happens to have an illegal stream index (i.e. one that
> does not correspond to a stream), it should nevertheless be
> unreferenced.
> 3. If putting a packet on a packet list fails, it wasn't unreferenced.
> 
> Furthermore, read_frame_internal leaked a packet's (side) data if a
> context update was required and failed.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> The two memleaks in read_frame_internal have been added in v2.
> 
>  libavformat/utils.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index b57e680089..564be02334 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -873,8 +873,10 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
>          }
>  
>          err = av_packet_make_refcounted(pkt);
> -        if (err < 0)
> +        if (err < 0) {
> +            av_packet_unref(pkt);
>              return err;
> +        }
>  
>          if ((s->flags & AVFMT_FLAG_DISCARD_CORRUPT) &&
>              (pkt->flags & AV_PKT_FLAG_CORRUPT)) {
> @@ -887,6 +889,7 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
>  
>          if (pkt->stream_index >= (unsigned)s->nb_streams) {
>              av_log(s, AV_LOG_ERROR, "Invalid stream index %d\n", pkt->stream_index);
> +            av_packet_unref(pkt);
>              continue;
>          }
>  
> @@ -917,8 +920,10 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
>          err = ff_packet_list_put(&s->internal->raw_packet_buffer,
>                                   &s->internal->raw_packet_buffer_end,
>                                   pkt, 0);
> -        if (err)
> +        if (err < 0) {
> +            av_packet_unref(pkt);
>              return err;
> +        }
>          s->internal->raw_packet_buffer_remaining_size -= pkt->size;
>  
>          if ((err = probe_codec(s, st, pkt)) < 0)
> @@ -1611,15 +1616,19 @@ static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
>              }
>  
>              ret = avcodec_parameters_to_context(st->internal->avctx, st->codecpar);
> -            if (ret < 0)
> +            if (ret < 0) {
> +                av_packet_unref(&cur_pkt);
>                  return ret;
> +            }
>  
>  #if FF_API_LAVF_AVCTX
>  FF_DISABLE_DEPRECATION_WARNINGS
>              /* update deprecated public codec context */
>              ret = avcodec_parameters_to_context(st->codec, st->codecpar);
> -            if (ret < 0)
> +            if (ret < 0) {
> +                av_packet_unref(&cur_pkt);
>                  return ret;
> +            }
>  FF_ENABLE_DEPRECATION_WARNINGS
>  #endif
>  
> 
Ping for the whole patchset. Notice that the commit message of #7
contains a typo; [1] is a fixed version.

- Andreas

[1]:
https://ffmpeg.org/pipermail/ffmpeg-devel/2019-September/249683.html
(Patchwork: https://patchwork.ffmpeg.org/patch/14991/)

Patch hide | download patch | download mbox

diff --git a/libavformat/utils.c b/libavformat/utils.c
index b57e680089..564be02334 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -873,8 +873,10 @@  int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
         }
 
         err = av_packet_make_refcounted(pkt);
-        if (err < 0)
+        if (err < 0) {
+            av_packet_unref(pkt);
             return err;
+        }
 
         if ((s->flags & AVFMT_FLAG_DISCARD_CORRUPT) &&
             (pkt->flags & AV_PKT_FLAG_CORRUPT)) {
@@ -887,6 +889,7 @@  int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
 
         if (pkt->stream_index >= (unsigned)s->nb_streams) {
             av_log(s, AV_LOG_ERROR, "Invalid stream index %d\n", pkt->stream_index);
+            av_packet_unref(pkt);
             continue;
         }
 
@@ -917,8 +920,10 @@  int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
         err = ff_packet_list_put(&s->internal->raw_packet_buffer,
                                  &s->internal->raw_packet_buffer_end,
                                  pkt, 0);
-        if (err)
+        if (err < 0) {
+            av_packet_unref(pkt);
             return err;
+        }
         s->internal->raw_packet_buffer_remaining_size -= pkt->size;
 
         if ((err = probe_codec(s, st, pkt)) < 0)
@@ -1611,15 +1616,19 @@  static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
             }
 
             ret = avcodec_parameters_to_context(st->internal->avctx, st->codecpar);
-            if (ret < 0)
+            if (ret < 0) {
+                av_packet_unref(&cur_pkt);
                 return ret;
+            }
 
 #if FF_API_LAVF_AVCTX
 FF_DISABLE_DEPRECATION_WARNINGS
             /* update deprecated public codec context */
             ret = avcodec_parameters_to_context(st->codec, st->codecpar);
-            if (ret < 0)
+            if (ret < 0) {
+                av_packet_unref(&cur_pkt);
                 return ret;
+            }
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif