On 3/6/2018 11:03 PM, James Almer wrote:
> On 3/6/2018 10:47 PM, Michael Niedermayer wrote:
>> On Tue, Mar 06, 2018 at 01:42:36AM -0300, James Almer wrote:
>>> This prevents leaks in the rare cases the function is called when extradata
>>> already exists.
>>>
>>> Signed-off-by: James Almer <jamr...@gmail.com>
>>> ---
>>> libavformat/utils.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>>> index 72531d4185..31340a484b 100644
>>> --- a/libavformat/utils.c
>>> +++ b/libavformat/utils.c
>>> @@ -3245,6 +3245,7 @@ int ff_alloc_extradata(AVCodecParameters *par, int
>>> size)
>>> {
>>> int ret;
>>>
>>> + av_freep(&par->extradata);
>>> if (size < 0 || size >= INT32_MAX - AV_INPUT_BUFFER_PADDING_SIZE) {
>>> par->extradata = NULL;
>>> par->extradata_size = 0;
>>
>> This causes memory corruption
>> ...
>> [mpegts @ 0x7f8c74000a80] PES packet size mismatch
>> *** Error in `./ffplay': double free or corruption (fasttop):
>> 0x00007f8c7402d9c0 ***
>> Aborted (core dumped)
>
> Is something freeing extradata and leaving a dangling pointer before
> eventually calling ff_alloc_extradata()?
> At least the two calls in mpegts.c don't seem to do that.
>
>>
>> I think this should not have been applied so quickly, i tested it as soon as
>> i
>> had time and saw it but it was applied already
>
> Fate passed when i tested it, and i got a positive review from the
> author of the function in question, that's why i pushed it. Sorry.
>
>>
>> If it helps i can debug the cases i see to find out which calls cause them
>> but
>> someone will still have to review all call sites probably for this change to
>> be safe
>
> Yes, help would be welcome. Crashes like this probably hint at frees
> leaving dangling pointers across the codebase.
Does the attached patch fix this crash?
From 1f128144cdd2f212b2ec74b29e2db771ad0977a6 Mon Sep 17 00:00:00 2001
From: James Almer <jamr...@gmail.com>
Date: Tue, 6 Mar 2018 23:26:22 -0300
Subject: [PATCH] avformat/isom: don't free extradata before calling
ff_get_extradata()
Using av_free() leaves a dangling pointer.
Signed-off-by: James Almer <jamr...@gmail.com>
---
libavformat/isom.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/libavformat/isom.c b/libavformat/isom.c
index 59502a8b3f..2792371c25 100644
--- a/libavformat/isom.c
+++ b/libavformat/isom.c
@@ -526,7 +526,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
av_log(fc, AV_LOG_TRACE, "Specific MPEG-4 header len=%d\n", len);
if (!len || (uint64_t)len > (1<<30))
return AVERROR_INVALIDDATA;
- av_free(st->codecpar->extradata);
if ((ret = ff_get_extradata(fc, st->codecpar, pb, len)) < 0)
return ret;
if (st->codecpar->codec_id == AV_CODEC_ID_AAC) {
--
2.16.2
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
