[FFmpeg-devel] [PATCH] avformat/mp3dec, rmdec: check return value of ffio_ensure_seekback
Hendrik Leppkes
h.leppkes at gmail.com
Mon Nov 16 13:53:17 CET 2015
On Mon, Nov 16, 2015 at 1:52 PM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> On Mon, Nov 16, 2015 at 3:27 AM, wm4 <nfxjfg at gmail.com> wrote:
>> On Sun, 15 Nov 2015 17:56:22 -0500
>> Ganesh Ajjanagadde <gajjanagadde at gmail.com> wrote:
>>
>>> ffio_ensure_seekback can fail due to e.g ENOMEM. This return value is
>>> propagated here, and all usage in the codebase now has its return value
>>> checked.
>>>
>>> A potential memory leak in mp3_read_header is also fixed via a goto
>>> fail.
>>>
>>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>>> ---
>>> libavformat/mp3dec.c | 12 +++++++++---
>>> libavformat/rmdec.c | 3 ++-
>>> 2 files changed, 11 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
>>> index 32ca00c..9fefe2d 100644
>>> --- a/libavformat/mp3dec.c
>>> +++ b/libavformat/mp3dec.c
>>> @@ -373,18 +373,20 @@ static int mp3_read_header(AVFormatContext *s)
>>>
>>> ret = ff_replaygain_export(st, s->metadata);
>>> if (ret < 0)
>>> - return ret;
>>> + goto fail;
>>>
>>> off = avio_tell(s->pb);
>>> for (i = 0; i < 64 * 1024; i++) {
>>> uint32_t header, header2;
>>> int frame_size;
>>> if (!(i&1023))
>>> - ffio_ensure_seekback(s->pb, i + 1024 + 4);
>>> + if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + 4)) < 0)
>>> + goto fail;
>>> frame_size = check(s->pb, off + i, &header);
>>> if (frame_size > 0) {
>>> avio_seek(s->pb, off, SEEK_SET);
>>> - ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4);
>>> + if ((ret = ffio_ensure_seekback(s->pb, i + 1024 + frame_size + 4)) < 0)
>>> + goto fail;
>>> if (check(s->pb, off + i + frame_size, &header2) >= 0 &&
>>> (header & SAME_HEADER_MASK) == (header2 & SAME_HEADER_MASK))
>>> {
>>> @@ -402,6 +404,10 @@ static int mp3_read_header(AVFormatContext *s)
>>>
>>> /* the parameters will be extracted from the compressed bitstream */
>>> return 0;
>>> +
>>> +fail:
>>> + ff_free_stream(s, st);
>>> + return ret;
>>> }
>>>
>>> #define MP3_PACKET_SIZE 1024
>>> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
>>> index 4ec78ef..d6e820e 100644
>>> --- a/libavformat/rmdec.c
>>> +++ b/libavformat/rmdec.c
>>> @@ -576,7 +576,8 @@ static int rm_read_header(AVFormatContext *s)
>>> size = avio_rb32(pb);
>>> codec_pos = avio_tell(pb);
>>>
>>> - ffio_ensure_seekback(pb, 4);
>>> + if ((ret = ffio_ensure_seekback(pb, 4)) < 0)
>>> + goto fail;
>>> v = avio_rb32(pb);
>>> if (v == MKBETAG('M', 'L', 'T', 'I')) {
>>> int number_of_streams = avio_rb16(pb);
>>
>> NACK. There's no reason to fatally fail in these cases.
>
> Ok, will split into two for the memory leak and these return values.
> For the return values, will simply log at AV_LOG_WARNING.
>
There is no actual memory leak here, the stream is free'ed when the
format context is closed.
- Hendrik
More information about the ffmpeg-devel
mailing list