[FFmpeg-devel] [PATCH] pass MpegTSContext ptr explicitly when it's needed (fixes #3721)
Marton Balint
cus at passwd.hu
Tue Jul 8 21:18:08 CEST 2014
On Tue, 8 Jul 2014, Alexander Lukyanov wrote:
> I have tested with reduced changes and I can confirm that #3721 is really
> fixed by them. Updated patch is attached.
>
LGTM, thanks.
Marton
>
>
> 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.
>
More information about the ffmpeg-devel
mailing list