diff mbox series

[FFmpeg-devel,2/3] avformat/swfdec: Reorder allocations/initializations

Message ID 20200920115341.1812809-2-andreas.rheinhardt@gmail.com
State Accepted
Commit 3f04c3037223f5e5417a14674103f3eeabb4887c
Headers show
Series [FFmpeg-devel,1/3] avformat/swfdec: Fix memleaks on error
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Sept. 20, 2020, 11:53 a.m. UTC
The earlier code would first attempt to allocate two buffers, then
attempt to allocate an AVIOContext, using one of the new buffers I/O
buffer, then check the allocations. On success, a z_stream that is used
in the AVIOContext's read_packet callback is initialized afterwards.

There are two problems with this: In case the allocation of the I/O
buffer fails avio_alloc_context() will be given a NULL read buffer
with a size > 0. This works right now, but it is fragile. The second
problem is that the z_stream used in the read_packet callback is not
functional when avio_alloc_context() is allocated (it might be that
avio_alloc_context() might already fill the buffer in the future). This
commit fixes both of these problems by reordering the operations.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/swfdec.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

Comments

Andreas Rheinhardt Sept. 23, 2020, 12:10 p.m. UTC | #1
Andreas Rheinhardt:
> The earlier code would first attempt to allocate two buffers, then
> attempt to allocate an AVIOContext, using one of the new buffers I/O
> buffer, then check the allocations. On success, a z_stream that is used
> in the AVIOContext's read_packet callback is initialized afterwards.
> 
> There are two problems with this: In case the allocation of the I/O
> buffer fails avio_alloc_context() will be given a NULL read buffer
> with a size > 0. This works right now, but it is fragile. The second
> problem is that the z_stream used in the read_packet callback is not
> functional when avio_alloc_context() is allocated (it might be that
> avio_alloc_context() might already fill the buffer in the future). This
> commit fixes both of these problems by reordering the operations.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/swfdec.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/libavformat/swfdec.c b/libavformat/swfdec.c
> index 7a74fa3ac7..d1ed1e2a53 100644
> --- a/libavformat/swfdec.c
> +++ b/libavformat/swfdec.c
> @@ -128,6 +128,8 @@ retry:
>  
>      return buf_size - z->avail_out;
>  }
> +
> +static av_cold int swf_read_close(AVFormatContext *avctx);
>  #endif
>  
>  static int swf_read_header(AVFormatContext *s)
> @@ -142,24 +144,18 @@ static int swf_read_header(AVFormatContext *s)
>      if (tag == MKBETAG('C', 'W', 'S', 0)) {
>          av_log(s, AV_LOG_INFO, "SWF compressed file detected\n");
>  #if CONFIG_ZLIB
> -        swf->zbuf_in  = av_malloc(ZBUF_SIZE);
> -        swf->zbuf_out = av_malloc(ZBUF_SIZE);
> -        swf->zpb = avio_alloc_context(swf->zbuf_out, ZBUF_SIZE, 0, s,
> -                                      zlib_refill, NULL, NULL);
> -        if (!swf->zbuf_in || !swf->zbuf_out || !swf->zpb) {
> -            av_freep(&swf->zbuf_in);
> -            av_freep(&swf->zbuf_out);
> -            avio_context_free(&swf->zpb);
> -            return AVERROR(ENOMEM);
> -        }
> -        swf->zpb->seekable = 0;
>          if (inflateInit(&swf->zstream) != Z_OK) {
>              av_log(s, AV_LOG_ERROR, "Unable to init zlib context\n");
> -            av_freep(&swf->zbuf_in);
> -            av_freep(&swf->zbuf_out);
> -            avio_context_free(&swf->zpb);
>              return AVERROR(EINVAL);
>          }
> +        if (!(swf->zbuf_in  = av_malloc(ZBUF_SIZE)) ||
> +            !(swf->zbuf_out = av_malloc(ZBUF_SIZE)) ||
> +            !(swf->zpb = avio_alloc_context(swf->zbuf_out, ZBUF_SIZE, 0,
> +                                            s, zlib_refill, NULL, NULL))) {
> +            swf_read_close(s);
> +            return AVERROR(ENOMEM);
> +        }
> +        swf->zpb->seekable = 0;
>          pb = swf->zpb;
>  #else
>          av_log(s, AV_LOG_ERROR, "zlib support is required to read SWF compressed files\n");
> 
Will apply.

- Andreas
diff mbox series

Patch

diff --git a/libavformat/swfdec.c b/libavformat/swfdec.c
index 7a74fa3ac7..d1ed1e2a53 100644
--- a/libavformat/swfdec.c
+++ b/libavformat/swfdec.c
@@ -128,6 +128,8 @@  retry:
 
     return buf_size - z->avail_out;
 }
+
+static av_cold int swf_read_close(AVFormatContext *avctx);
 #endif
 
 static int swf_read_header(AVFormatContext *s)
@@ -142,24 +144,18 @@  static int swf_read_header(AVFormatContext *s)
     if (tag == MKBETAG('C', 'W', 'S', 0)) {
         av_log(s, AV_LOG_INFO, "SWF compressed file detected\n");
 #if CONFIG_ZLIB
-        swf->zbuf_in  = av_malloc(ZBUF_SIZE);
-        swf->zbuf_out = av_malloc(ZBUF_SIZE);
-        swf->zpb = avio_alloc_context(swf->zbuf_out, ZBUF_SIZE, 0, s,
-                                      zlib_refill, NULL, NULL);
-        if (!swf->zbuf_in || !swf->zbuf_out || !swf->zpb) {
-            av_freep(&swf->zbuf_in);
-            av_freep(&swf->zbuf_out);
-            avio_context_free(&swf->zpb);
-            return AVERROR(ENOMEM);
-        }
-        swf->zpb->seekable = 0;
         if (inflateInit(&swf->zstream) != Z_OK) {
             av_log(s, AV_LOG_ERROR, "Unable to init zlib context\n");
-            av_freep(&swf->zbuf_in);
-            av_freep(&swf->zbuf_out);
-            avio_context_free(&swf->zpb);
             return AVERROR(EINVAL);
         }
+        if (!(swf->zbuf_in  = av_malloc(ZBUF_SIZE)) ||
+            !(swf->zbuf_out = av_malloc(ZBUF_SIZE)) ||
+            !(swf->zpb = avio_alloc_context(swf->zbuf_out, ZBUF_SIZE, 0,
+                                            s, zlib_refill, NULL, NULL))) {
+            swf_read_close(s);
+            return AVERROR(ENOMEM);
+        }
+        swf->zpb->seekable = 0;
         pb = swf->zpb;
 #else
         av_log(s, AV_LOG_ERROR, "zlib support is required to read SWF compressed files\n");