| 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 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
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 --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");
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(-)