[FFmpeg-devel] [PATCH] fix segfault in mxf demuxer

[Nigel Touati-Evans]

On 22 March 2011 19:25, Baptiste Coudurier <baptiste.coudurier at gmail.com> wrote:
> On 3/22/11 3:30 AM, Nigel Touati-Evans wrote:
>> On 21 March 2011 20:09, Baptiste Coudurier <baptiste.coudurier at gmail.com> wrote:
>>> Hi,
>>>
>>> On 03/21/2011 05:40 AM, Tomas Härdin wrote:
>>>> Nigel Touati-Evans skrev 2011-03-21 12:06:
>>>>> Index: ffmpeg-dmo-0.5+svn20090508/libavformat/mxfdec.c
>>>>> ===================================================================
>>>>> --- ffmpeg-dmo-0.5+svn20090508.orig/libavformat/mxfdec.c    2011-03-21
>>>>> 10:12:47.000000000 +0000
>>>>> +++ ffmpeg-dmo-0.5+svn20090508/libavformat/mxfdec.c    2011-03-21
>>>>> 10:17:13.000000000 +0000
>>>>> @@ -723,13 +723,16 @@
>>>>>                      break;
>>>>>                  }
>>>>>              }
>>>>> -            if (!source_track) {
>>>>> -                av_log(mxf->fc, AV_LOG_ERROR, "material track %d: no
>>>>> corresponding source track found\n", material_track->track_id);
>>>>> +            if (source_track) {
>>>>> +                if (j+1 <
>>>>> material_track->sequence->structural_components_count)
>>>>> +                   av_log(mxf->fc, AV_LOG_WARNING, "material track
>>>>> %d: found a source track so ignoring %d potential others\n",
>>>>> material_track->track_id,
>>>>> material_track->sequence->structural_components_count-j-1);
>>>>
>>>> Break this long line up a bit.
>>>>
>>>>>                  break;
>>>>>              }
>>>>>          }
>>>>> -        if (!source_track)
>>>>> +        if (!source_track) {
>>>>> +            av_log(mxf->fc, AV_LOG_ERROR, "material track %d: no
>>>>> corresponding source track found\n", material_track->track_id);
>>>>>              continue;
>>>>> +        }
>>>>>
>>>>>          st = av_new_stream(mxf->fc, source_track->track_id);
>>>>>          if (!st) {
>>>>>
>>>>>
>>>>
>>>> Looks OK otherwise to me. Baptiste?
>>>
>>> Can we get a sample so I can reproduce the problem ?
>>> I'd like to know where it exactly segv.
>>>
>>> --
>>> Baptiste COUDURIER
>>> Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
>>> FFmpeg maintainer                                  http://www.ffmpeg.org
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>
>> I'm not sure I have permission to upload the file - I'll try to find
>> out. However its quite simple to see how the segv happens:
>>
>> For this track sequence->structural_components_count is 2, the first
>> time round (j=0) the loop component is set and a source track found.
>> When j=1, mxf_resolve_strong_ref returns null (the component is either
>> not found or the not of the correct type), so the loop exits. This
>> leaves source_track set and component null, so there is a seg fault
>> trying to dereference component to find the duration.
>>
>> In general it doesn't look like the loop ensures the source_track
>> corresponds to the component, as component is updated every iteration
>> and source_track is not necessarily (e.g. if it is not found), so it
>> seemed to me that the best bet was to exit the loop once a suitable
>> component had been found.
>>
>> This doesn't fix the 'TODO: handle multiple source clips' but at least
>> makes everything consistent when structural_components_count > 1, and
>> prints a warning if we might be missing something.
>
> Thanks, patch ok.
>
> --
> Baptiste COUDURIER
> Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
> FFmpeg maintainer                                  http://www.ffmpeg.org
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Thanks - do you want me to attach a new patch with that long line split up?

Nigel