Message ID | 20190819215624.49795-1-andreas.rheinhardt@gmail.com |
---|---|
State | Accepted |
Headers | show |
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/)
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
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(-)