[FFmpeg-devel] [PATCH] pass MpegTSContext ptr explicitly when it's needed (fixes #3721)
Alexander Lukyanov
lavv17f at gmail.com
Tue Jul 8 10:41:20 CEST 2014
I have tested with reduced changes and I can confirm that #3721 is really
fixed by them. Updated patch is attached.
2014-07-07 23:24 GMT+04:00 Marton Balint <cus at passwd.hu>:
>
>
> On Mon, 7 Jul 2014, Alexander Lukyanov wrote:
>
> The remaining parts were changed for consistency. As priv_data is not
>> always MpegTSContext, it is illegal to cast it without certain checks.
>>
>
> I believe for the remaining parts priv_data is always MpegTSContext,
> therefore it is safe to cast to it. If really that is the case, I don't see
> why we should change the remaining code.
>
>
>
>>
>> 2014-07-07 3:24 GMT+04:00 Marton Balint <cus at passwd.hu>:
>>
>>
>>>
>>> On Sun, 6 Jul 2014, Alexander V. Lukyanov wrote:
>>>
>>> From: "Alexander V. Lukyanov" <lavv17f at gmail.com>
>>>
>>>>
>>>> AVFormatContext->priv_data is not always a MpegTSContext, it can be
>>>> RTSPState when decoding a RTP stream. So it is necessary to pass
>>>> MpegTSContext pointer explicitly along with AVFormatContext (or alone).
>>>>
>>>> This fixes memory corruption from bug #3721 (RTSPState is smaller than
>>>> MpegTSContext thus innocent memory gets overwritten).
>>>>
>>>> Signed-off-by: Alexander V. Lukyanov <lavv17f at gmail.com>
>>>> ---
>>>> libavformat/mpegts.c | 26 ++++++++++++--------------
>>>> 1 file changed, 12 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
>>>> index 3434341..6dc5d77 100644
>>>> --- a/libavformat/mpegts.c
>>>> +++ b/libavformat/mpegts.c
>>>> @@ -357,10 +357,9 @@ static int discard_pid(MpegTSContext *ts, unsigned
>>>> int pid)
>>>> * Assemble PES packets out of TS packets, and then call the
>>>> "section_cb"
>>>> * function when they are complete.
>>>> */
>>>> -static void write_section_data(AVFormatContext *s, MpegTSFilter *tss1,
>>>> +static void write_section_data(MpegTSContext *ts, MpegTSFilter *tss1,
>>>> const uint8_t *buf, int buf_size, int
>>>> is_start)
>>>> {
>>>> - MpegTSContext *ts = s->priv_data;
>>>> MpegTSSectionFilter *tss = &tss1->u.section_filter;
>>>> int len;
>>>>
>>>> @@ -2010,7 +2009,6 @@ static int parse_pcr(int64_t *ppcr_high, int
>>>> *ppcr_low,
>>>> /* handle one TS packet */
>>>> static int handle_packet(MpegTSContext *ts, const uint8_t *packet)
>>>> {
>>>> - AVFormatContext *s = ts->stream;
>>>> MpegTSFilter *tss;
>>>> int len, pid, cc, expected_cc, cc_ok, afc, is_start,
>>>> is_discontinuity,
>>>> has_adaptation, has_payload;
>>>> @@ -2084,7 +2082,7 @@ static int handle_packet(MpegTSContext *ts, const
>>>> uint8_t *packet)
>>>> return 0;
>>>> if (len && cc_ok) {
>>>> /* write remaining section bytes */
>>>> - write_section_data(s, tss,
>>>> + write_section_data(ts, tss,
>>>> p, len, 0);
>>>> /* check whether filter has been closed */
>>>> if (!ts->pids[pid])
>>>> @@ -2092,12 +2090,12 @@ static int handle_packet(MpegTSContext *ts,
>>>> const
>>>> uint8_t *packet)
>>>> }
>>>> p += len;
>>>> if (p < p_end) {
>>>> - write_section_data(s, tss,
>>>> + write_section_data(ts, tss,
>>>> p, p_end - p, 1);
>>>> }
>>>> } else {
>>>> if (cc_ok) {
>>>> - write_section_data(s, tss,
>>>> + write_section_data(ts, tss,
>>>> p, p_end - p, 0);
>>>> }
>>>> }
>>>>
>>>>
>>> So far, the patch is OK. But do you really need the remaining hunks in
>>> order to fix the crash? Because as far as I see, the remaining hunks
>>> affects code that is not used by the parsing functions. Or am I missing
>>> something?
>>>
>>>
>>> @@ -2170,7 +2168,7 @@ static void reanalyze(MpegTSContext *ts) {
>>>
>>>>
>>>> /* XXX: try to find a better synchro over several packets (use
>>>> * get_packet_size() ?) */
>>>> -static int mpegts_resync(AVFormatContext *s)
>>>> +static int mpegts_resync(AVFormatContext *s, MpegTSContext *ts)
>>>> {
>>>> AVIOContext *pb = s->pb;
>>>> int c, i;
>>>> @@ -2181,7 +2179,7 @@ static int mpegts_resync(AVFormatContext *s)
>>>> return AVERROR_EOF;
>>>> if (c == 0x47) {
>>>> avio_seek(pb, -1, SEEK_CUR);
>>>> - reanalyze(s->priv_data);
>>>> + reanalyze(ts);
>>>> return 0;
>>>> }
>>>> }
>>>> @@ -2192,7 +2190,7 @@ static int mpegts_resync(AVFormatContext *s)
>>>> }
>>>>
>>>> /* return AVERROR_something if error or EOF. Return 0 if OK. */
>>>> -static int read_packet(AVFormatContext *s, uint8_t *buf, int
>>>> raw_packet_size,
>>>> +static int read_packet(AVFormatContext *s, MpegTSContext *ts, uint8_t
>>>> *buf, int raw_packet_size,
>>>> const uint8_t **data)
>>>> {
>>>> AVIOContext *pb = s->pb;
>>>> @@ -2208,7 +2206,7 @@ static int read_packet(AVFormatContext *s, uint8_t
>>>> *buf, int raw_packet_size,
>>>> uint64_t pos = avio_tell(pb);
>>>> avio_seek(pb, -FFMIN(raw_packet_size, pos), SEEK_CUR);
>>>>
>>>> - if (mpegts_resync(s) < 0)
>>>> + if (mpegts_resync(s, ts) < 0)
>>>> return AVERROR(EAGAIN);
>>>> else
>>>> continue;
>>>> @@ -2265,7 +2263,7 @@ static int handle_packets(MpegTSContext *ts, int
>>>> nb_packets)
>>>> if (ts->stop_parse > 0)
>>>> break;
>>>>
>>>> - ret = read_packet(s, packet, ts->raw_packet_size, &data);
>>>> + ret = read_packet(s, ts, packet, ts->raw_packet_size, &data);
>>>> if (ret != 0)
>>>> break;
>>>> ret = handle_packet(ts, data);
>>>> @@ -2409,7 +2407,7 @@ static int mpegts_read_header(AVFormatContext *s)
>>>> nb_pcrs = 0;
>>>> nb_packets = 0;
>>>> for (;;) {
>>>> - ret = read_packet(s, packet, ts->raw_packet_size, &data);
>>>> + ret = read_packet(s, ts, packet, ts->raw_packet_size,
>>>> &data);
>>>> if (ret < 0)
>>>> return ret;
>>>> pid = AV_RB16(data + 1) & 0x1fff;
>>>> @@ -2456,7 +2454,7 @@ static int mpegts_raw_read_packet(AVFormatContext
>>>> *s, AVPacket *pkt)
>>>>
>>>> if (av_new_packet(pkt, TS_PACKET_SIZE) < 0)
>>>> return AVERROR(ENOMEM);
>>>> - ret = read_packet(s, pkt->data, ts->raw_packet_size, &data);
>>>> + ret = read_packet(s, ts, pkt->data, ts->raw_packet_size, &data);
>>>> pkt->pos = avio_tell(s->pb);
>>>> if (ret < 0) {
>>>> av_free_packet(pkt);
>>>> @@ -2558,7 +2556,7 @@ static av_unused int64_t
>>>> mpegts_get_pcr(AVFormatContext *s, int stream_index,
>>>> return AV_NOPTS_VALUE;
>>>> if (buf[0] != 0x47) {
>>>> avio_seek(s->pb, -TS_PACKET_SIZE, SEEK_CUR);
>>>> - if (mpegts_resync(s) < 0)
>>>> + if (mpegts_resync(s, ts) < 0)
>>>> return AV_NOPTS_VALUE;
>>>> pos = avio_tell(s->pb);
>>>> continue;
>>>> --
>>>> 1.8.3.1
>>>>
>>>>
>>>> Thanks,
>>> Marton
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>>
>>
>>
>> --
>> Alexander.
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>>
> Regards,
>
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
--
Alexander.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avformat-mpegts-pass-MpegTSContext-ptr-explicitly-fi.patch
Type: text/x-patch
Size: 2695 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140708/6791a75f/attachment.bin>
More information about the ffmpeg-devel
mailing list