[FFmpeg-devel] [PATCH] pass MpegTSContext ptr explicitly when it's needed (fixes #3721)
Alexander Lukyanov
lavv17f at gmail.com
Mon Jul 7 09:58:44 CEST 2014
The remaining parts were changed for consistency. As priv_data is not
always MpegTSContext, it is illegal to cast it without certain checks.
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.
More information about the ffmpeg-devel
mailing list