Michael Niedermayer: > On Sun, Jul 19, 2020 at 07:55:24PM +0200, Andreas Rheinhardt wrote: >> James Almer: >>> On 7/19/2020 2:42 PM, Michael Niedermayer wrote: >>>> Fixes: memleak >>>> Fixes: >>>> 23660/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6007508031504384 >>>> >>>> Found-by: continuous fuzzing process >>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>>> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> >>>> --- >>>> libavformat/wc3movie.c | 32 +++++++++++++++++++++++--------- >>>> 1 file changed, 23 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/libavformat/wc3movie.c b/libavformat/wc3movie.c >>>> index c59b5bf6cc..76e945d261 100644 >>>> --- a/libavformat/wc3movie.c >>>> +++ b/libavformat/wc3movie.c >>>> @@ -139,10 +139,14 @@ static int wc3_read_header(AVFormatContext *s) >>>> /* load up the name */ >>>> buffer = av_malloc(size+1); >>>> if (!buffer) >>>> - return AVERROR(ENOMEM); >>>> + if (!buffer) { >>>> + ret = AVERROR(ENOMEM); >>>> + goto fail; >>>> + } >>>> if ((ret = avio_read(pb, buffer, size)) != size) { >>>> av_freep(&buffer); >>>> - return AVERROR(EIO); >>>> + ret = AVERROR(EIO); >>>> + goto fail; >>>> } >>>> buffer[size] = 0; >>>> av_dict_set(&s->metadata, "title", buffer, >>>> @@ -164,21 +168,26 @@ static int wc3_read_header(AVFormatContext *s) >>>> default: >>>> av_log(s, AV_LOG_ERROR, "unrecognized WC3 chunk: %s\n", >>>> av_fourcc2str(fourcc_tag)); >>>> - return AVERROR_INVALIDDATA; >>>> + ret = AVERROR_INVALIDDATA; >>>> + goto fail; >>>> } >>>> >>>> fourcc_tag = avio_rl32(pb); >>>> /* chunk sizes are 16-bit aligned */ >>>> size = (avio_rb32(pb) + 1) & (~1); >>>> - if (avio_feof(pb)) >>>> - return AVERROR(EIO); >>>> + if (avio_feof(pb)) { >>>> + ret = AVERROR(EIO); >>>> + goto fail; >>>> + } >>>> >>>> } while (fourcc_tag != BRCH_TAG); >>>> >>>> /* initialize the decoder streams */ >>>> st = avformat_new_stream(s, NULL); >>>> - if (!st) >>>> - return AVERROR(ENOMEM); >>>> + if (!st) { >>>> + ret = AVERROR(ENOMEM); >>>> + goto fail; >>>> + } >>>> avpriv_set_pts_info(st, 33, 1, WC3_FRAME_FPS); >>>> wc3->video_stream_index = st->index; >>>> st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; >>>> @@ -188,8 +197,10 @@ static int wc3_read_header(AVFormatContext *s) >>>> st->codecpar->height = wc3->height; >>>> >>>> st = avformat_new_stream(s, NULL); >>>> - if (!st) >>>> - return AVERROR(ENOMEM); >>>> + if (!st) { >>>> + ret = AVERROR(ENOMEM); >>>> + goto fail; >>>> + } >>>> avpriv_set_pts_info(st, 33, 1, WC3_FRAME_FPS); >>>> wc3->audio_stream_index = st->index; >>>> st->codecpar->codec_type = AVMEDIA_TYPE_AUDIO; >>>> @@ -204,6 +215,9 @@ static int wc3_read_header(AVFormatContext *s) >>>> st->codecpar->block_align = WC3_AUDIO_BITS * WC3_AUDIO_CHANNELS; >>>> >>>> return 0; >>>> +fail: >>>> + wc3_read_close(s); >>> >>> Wouldn't it be better to instead make avformat_open_input() call >>> iformat->read_close() on iformat->read_header() failure? >>> >>> It may require ensuring all demuxers behave nice with it, but the end >>> result would be a lot cleaner. >>> >> >> Problem is: Not all input devices behave nice and it is possible to use >> an older libavdevice together with a newer libavformat. You might >> remember the patchset where I added a flag to AVInputFormat for this >> purpose. I'll resend it soon. > > 2 months have passed, the memleak is still open and i dont see a flag or > init/deinit() for demuxers. > So i suggest to apply this patch as it was. A flag is better but a leak is > worst. > I agree.
- Andreas _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
