[FFmpeg-devel] [PATCH] avformat/mpegts: Shuffle avio_seek

[Marton Balint]

On Thu, 7 May 2020, Michael Niedermayer wrote:

> On Thu, May 07, 2020 at 02:09:43PM +0200, Anton Khirnov wrote:
>> Quoting Michael Niedermayer (2020-05-07 12:38:26)
>>> This avoids accessing an old, no longer valid buffer.
>>> Fixes: out of array access
>>> Fixes: crash_audio-2020
>>>
>>> Found-by: le wu <shoulewoba at gmail.com>
>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>> ---
>>>  libavformat/mpegts.c | 7 ++++---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
>>> index 0833d62ea5..a065c61c40 100644
>>> --- a/libavformat/mpegts.c
>>> +++ b/libavformat/mpegts.c
>>> @@ -2881,15 +2881,16 @@ static int mpegts_resync(AVFormatContext *s, int seekback, const uint8_t *curren
>>>      AVIOContext *pb = s->pb;
>>>      int c, i;
>>>      uint64_t pos = avio_tell(pb);
>>> -
>>> -    avio_seek(pb, -FFMIN(seekback, pos), SEEK_CUR);
>>> +    int64_t back = FFMIN(seekback, pos);
>>>
>>>      //Special case for files like 01c56b0dc1.ts
>>>      if (current_packet[0] == 0x80 && current_packet[12] == 0x47) {
>>> -        avio_seek(pb, 12, SEEK_CUR);
>>> +        avio_seek(pb, 12 - back, SEEK_CUR);
>>>          return 0;
>>>      }
>>>
>>> +    avio_seek(pb, -back, SEEK_CUR);
>>> +
>>
>> This seems pretty non-obvious - why would ordering seeks in a specific
>> way result in invalid memorry access?
>
> because current_packet in one case points to the avio internal buffer
> and doing any "state changing" avio could change that buffer.
> so all accesses to the buffer must happen before any avio seeks

Yuck. Patch LGTM though.

Thanks,
Marton