diff mbox series

[FFmpeg-devel,09/11] avformat/utils: Fix memleaks in avformat_open_input()

Message ID 20200107135549.22581-9-andreas.rheinhardt@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel,01/11] avformat/avformat: Update AVInputFormat.read_packet documentation
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt Jan. 7, 2020, 1:55 p.m. UTC
A demuxer might have allocated memory while reading the header. If
reading the header was successfull and an error happens before returning
(e.g. when queueing the attached pictures), the read_close function
would have never been called, so that all those allocations would leak.
This commit changes this.

Furthermore, there would be even more memleaks if the error level was
set to AV_EF_EXPLODE in case there is both metadata and id3v2 metadata.
This has been fixed, too.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/utils.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

Comments

Marton Balint Feb. 15, 2020, 5:38 p.m. UTC | #1
On Tue, 7 Jan 2020, Andreas Rheinhardt wrote:

> A demuxer might have allocated memory while reading the header. If
> reading the header was successfull and an error happens before returning
> (e.g. when queueing the attached pictures), the read_close function
> would have never been called, so that all those allocations would leak.
> This commit changes this.
>
> Furthermore, there would be even more memleaks if the error level was
> set to AV_EF_EXPLODE in case there is both metadata and id3v2 metadata.
> This has been fixed, too.

Thanks, applied.

Regards,
Marton

>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> libavformat/utils.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 2470a6ac0e..cf7dabefbb 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -642,26 +642,28 @@ FF_ENABLE_DEPRECATION_WARNINGS
>             level = AV_LOG_ERROR;
>         av_log(s, level, "Discarding ID3 tags because more suitable tags were found.\n");
>         av_dict_free(&s->internal->id3v2_meta);
> -        if (s->error_recognition & AV_EF_EXPLODE)
> -            return AVERROR_INVALIDDATA;
> +        if (s->error_recognition & AV_EF_EXPLODE) {
> +            ret = AVERROR_INVALIDDATA;
> +            goto close;
> +        }
>     }
>
>     if (id3v2_extra_meta) {
>         if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, "aac") ||
>             !strcmp(s->iformat->name, "tta") || !strcmp(s->iformat->name, "wav")) {
>             if ((ret = ff_id3v2_parse_apic(s, &id3v2_extra_meta)) < 0)
> -                goto fail;
> +                goto close;
>             if ((ret = ff_id3v2_parse_chapters(s, &id3v2_extra_meta)) < 0)
> -                goto fail;
> +                goto close;
>             if ((ret = ff_id3v2_parse_priv(s, &id3v2_extra_meta)) < 0)
> -                goto fail;
> +                goto close;
>         } else
>             av_log(s, AV_LOG_DEBUG, "demuxer does not support additional id3 data, skipping\n");
>     }
>     ff_id3v2_free_extra_meta(&id3v2_extra_meta);
>
>     if ((ret = avformat_queue_attached_pictures(s)) < 0)
> -        goto fail;
> +        goto close;
>
>     if (!(s->flags&AVFMT_FLAG_PRIV_OPT) && s->pb && !s->internal->data_offset)
>         s->internal->data_offset = avio_tell(s->pb);
> @@ -680,6 +682,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
>     *ps = s;
>     return 0;
> 
> +close:
> +    if (s->iformat->read_close)
> +        s->iformat->read_close(s);
> fail:
>     ff_id3v2_free_extra_meta(&id3v2_extra_meta);
>     av_dict_free(&tmp);
> -- 
> 2.20.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavformat/utils.c b/libavformat/utils.c
index 2470a6ac0e..cf7dabefbb 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -642,26 +642,28 @@  FF_ENABLE_DEPRECATION_WARNINGS
             level = AV_LOG_ERROR;
         av_log(s, level, "Discarding ID3 tags because more suitable tags were found.\n");
         av_dict_free(&s->internal->id3v2_meta);
-        if (s->error_recognition & AV_EF_EXPLODE)
-            return AVERROR_INVALIDDATA;
+        if (s->error_recognition & AV_EF_EXPLODE) {
+            ret = AVERROR_INVALIDDATA;
+            goto close;
+        }
     }
 
     if (id3v2_extra_meta) {
         if (!strcmp(s->iformat->name, "mp3") || !strcmp(s->iformat->name, "aac") ||
             !strcmp(s->iformat->name, "tta") || !strcmp(s->iformat->name, "wav")) {
             if ((ret = ff_id3v2_parse_apic(s, &id3v2_extra_meta)) < 0)
-                goto fail;
+                goto close;
             if ((ret = ff_id3v2_parse_chapters(s, &id3v2_extra_meta)) < 0)
-                goto fail;
+                goto close;
             if ((ret = ff_id3v2_parse_priv(s, &id3v2_extra_meta)) < 0)
-                goto fail;
+                goto close;
         } else
             av_log(s, AV_LOG_DEBUG, "demuxer does not support additional id3 data, skipping\n");
     }
     ff_id3v2_free_extra_meta(&id3v2_extra_meta);
 
     if ((ret = avformat_queue_attached_pictures(s)) < 0)
-        goto fail;
+        goto close;
 
     if (!(s->flags&AVFMT_FLAG_PRIV_OPT) && s->pb && !s->internal->data_offset)
         s->internal->data_offset = avio_tell(s->pb);
@@ -680,6 +682,9 @@  FF_ENABLE_DEPRECATION_WARNINGS
     *ps = s;
     return 0;
 
+close:
+    if (s->iformat->read_close)
+        s->iformat->read_close(s);
 fail:
     ff_id3v2_free_extra_meta(&id3v2_extra_meta);
     av_dict_free(&tmp);