[FFmpeg-devel] [PATCH 5/8] ffmdec: break infinite resync loop
Hi, some broken files can lead to an endless resync loop, which is avoided by attached patch. Best regards, Andreas >From 5682a0cafbaf9339352f3147ef7c494dea47 Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun Date: Sun, 8 Mar 2015 23:29:42 +0100 Subject: [PATCH 5/8] ffmdec: break infinite resync loop If resyncing leads to the same position as previously, it will again lead to a resync attempt, resulting in an infinite loop. Signed-off-by: Andreas Cadhalpun --- libavformat/ffmdec.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/libavformat/ffmdec.c b/libavformat/ffmdec.c index 35e4c03..9cbd20c 100644 --- a/libavformat/ffmdec.c +++ b/libavformat/ffmdec.c @@ -82,6 +82,7 @@ static int ffm_read_data(AVFormatContext *s, FFMContext *ffm = s->priv_data; AVIOContext *pb = s->pb; int len, fill_size, size1, frame_offset, id; +int64_t last_pos = -1; size1 = size; while (size > 0) { @@ -101,9 +102,16 @@ static int ffm_read_data(AVFormatContext *s, avio_seek(pb, tell, SEEK_SET); } id = avio_rb16(pb); /* PACKET_ID */ -if (id != PACKET_ID) +if (id != PACKET_ID) { if (ffm_resync(s, id) < 0) return -1; +if (avio_tell(pb) == last_pos) { +av_log(s, AV_LOG_ERROR, + "breaking resync loop at pos %"PRIx64"\n", last_pos); +return AVERROR_INVALIDDATA; +} +last_pos = avio_tell(pb); +} fill_size = avio_rb16(pb); ffm->dts = avio_rb64(pb); frame_offset = avio_rb16(pb); -- 2.1.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/8] ffmdec: break infinite resync loop
On Mon, Mar 09, 2015 at 12:04:13AM +0100, Andreas Cadhalpun wrote: > Hi, > > some broken files can lead to an endless resync loop, which is > avoided by attached patch. > > Best regards, > Andreas > ffmdec.c | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > 048852d2d9b0c25157015a4befd76323fc4b2cc6 > 0005-ffmdec-break-infinite-resync-loop.patch > From 5682a0cafbaf9339352f3147ef7c494dea47 Mon Sep 17 00:00:00 2001 > From: Andreas Cadhalpun > Date: Sun, 8 Mar 2015 23:29:42 +0100 > Subject: [PATCH 5/8] ffmdec: break infinite resync loop > > If resyncing leads to the same position as previously, it will again > lead to a resync attempt, resulting in an infinite loop. iam not sure this is sufficient and loops over more than 1 resync point arent possible maybe its better to never allow resync to start before or at the previous resync point [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/8] ffmdec: break infinite resync loop
On 09.03.2015 03:13, Michael Niedermayer wrote: On Mon, Mar 09, 2015 at 12:04:13AM +0100, Andreas Cadhalpun wrote: Hi, some broken files can lead to an endless resync loop, which is avoided by attached patch. Best regards, Andreas ffmdec.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) 048852d2d9b0c25157015a4befd76323fc4b2cc6 0005-ffmdec-break-infinite-resync-loop.patch From 5682a0cafbaf9339352f3147ef7c494dea47 Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun Date: Sun, 8 Mar 2015 23:29:42 +0100 Subject: [PATCH 5/8] ffmdec: break infinite resync loop If resyncing leads to the same position as previously, it will again lead to a resync attempt, resulting in an infinite loop. iam not sure this is sufficient and loops over more than 1 resync point arent possible maybe its better to never allow resync to start before or at the previous resync point I don't think this is possible, but it shouldn't hurt to change the check to '<='. This forces the syncpoints to always increase. Best regards, Andreas >From aa2d0cc852fe51734d9c9fd318dc94ec4af3970f Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun Date: Sun, 8 Mar 2015 23:29:42 +0100 Subject: [PATCH 5/8] ffmdec: break infinite resync loop If resyncing leads to the same position as previously, it will again lead to a resync attempt, resulting in an infinite loop. Signed-off-by: Andreas Cadhalpun --- libavformat/ffmdec.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/libavformat/ffmdec.c b/libavformat/ffmdec.c index ee34e73..c54d1d6 100644 --- a/libavformat/ffmdec.c +++ b/libavformat/ffmdec.c @@ -82,6 +82,7 @@ static int ffm_read_data(AVFormatContext *s, FFMContext *ffm = s->priv_data; AVIOContext *pb = s->pb; int len, fill_size, size1, frame_offset, id; +int64_t last_pos = -1; size1 = size; while (size > 0) { @@ -101,9 +102,16 @@ static int ffm_read_data(AVFormatContext *s, avio_seek(pb, tell, SEEK_SET); } id = avio_rb16(pb); /* PACKET_ID */ -if (id != PACKET_ID) +if (id != PACKET_ID) { if (ffm_resync(s, id) < 0) return -1; +if (avio_tell(pb) <= last_pos) { +av_log(s, AV_LOG_ERROR, + "breaking resync loop at pos %"PRIx64"\n", last_pos); +return AVERROR_INVALIDDATA; +} +last_pos = avio_tell(pb); +} fill_size = avio_rb16(pb); ffm->dts = avio_rb64(pb); frame_offset = avio_rb16(pb); -- 2.1.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/8] ffmdec: break infinite resync loop
On Mon, Mar 09, 2015 at 12:17:38PM +0100, Andreas Cadhalpun wrote: > On 09.03.2015 03:13, Michael Niedermayer wrote: > >On Mon, Mar 09, 2015 at 12:04:13AM +0100, Andreas Cadhalpun wrote: > >>Hi, > >> > >>some broken files can lead to an endless resync loop, which is > >>avoided by attached patch. > >> > >>Best regards, > >>Andreas > > > >> ffmdec.c | 10 +- > >> 1 file changed, 9 insertions(+), 1 deletion(-) > >>048852d2d9b0c25157015a4befd76323fc4b2cc6 > >>0005-ffmdec-break-infinite-resync-loop.patch > >> From 5682a0cafbaf9339352f3147ef7c494dea47 Mon Sep 17 00:00:00 2001 > >>From: Andreas Cadhalpun > >>Date: Sun, 8 Mar 2015 23:29:42 +0100 > >>Subject: [PATCH 5/8] ffmdec: break infinite resync loop > >> > >>If resyncing leads to the same position as previously, it will again > >>lead to a resync attempt, resulting in an infinite loop. > > > >iam not sure this is sufficient and loops over more than 1 resync > >point arent possible > >maybe its better to never allow resync to start before > >or at the previous resync point > > I don't think this is possible, but it shouldn't hurt to change the > check to '<='. This forces the syncpoints to always increase. i was thinking more about limiting the backward seek before resync to the last resync position +1 if there was a previous resync so that resync which moves forward could not end before. This would avoid the failure and allow the demuxer to continue, or at least thats the idea [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Freedom in capitalist society always remains about the same as it was in ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/8] ffmdec: break infinite resync loop
On 09.03.2015 13:59, Michael Niedermayer wrote: i was thinking more about limiting the backward seek before resync to the last resync position +1 if there was a previous resync so that resync which moves forward could not end before. This would avoid the failure and allow the demuxer to continue, or at least thats the idea I see, new patch attached. The +1 is not necessary, because it always reads the PACKET_ID field before resyncing. Best regards, Andreas >From 6bcd1b4a906e2da39f6361db19826a337ac5a745 Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun Date: Mon, 9 Mar 2015 14:59:44 +0100 Subject: [PATCH] ffmdec: limit the backward seek to the last resync position If resyncing leads to the same position as previously, it will again lead to a resync attempt, resulting in an infinite loop. Thus don't seek back beyond the last syncpoint. Signed-off-by: Andreas Cadhalpun --- libavformat/ffmdec.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/libavformat/ffmdec.c b/libavformat/ffmdec.c index fc43668..ec91b6f 100644 --- a/libavformat/ffmdec.c +++ b/libavformat/ffmdec.c @@ -79,6 +79,7 @@ static int ffm_read_data(AVFormatContext *s, FFMContext *ffm = s->priv_data; AVIOContext *pb = s->pb; int len, fill_size, size1, frame_offset, id; +int64_t last_pos = -1; size1 = size; while (size > 0) { @@ -98,9 +99,11 @@ static int ffm_read_data(AVFormatContext *s, avio_seek(pb, tell, SEEK_SET); } id = avio_rb16(pb); /* PACKET_ID */ -if (id != PACKET_ID) +if (id != PACKET_ID) { if (ffm_resync(s, id) < 0) return -1; +last_pos = avio_tell(pb); +} fill_size = avio_rb16(pb); ffm->dts = avio_rb64(pb); frame_offset = avio_rb16(pb); @@ -114,7 +117,9 @@ static int ffm_read_data(AVFormatContext *s, if (!frame_offset) { /* This packet has no frame headers in it */ if (avio_tell(pb) >= ffm->packet_size * 3LL) { -avio_seek(pb, -ffm->packet_size * 2LL, SEEK_CUR); +int64_t seekback = FFMIN(ffm->packet_size * 2LL, avio_tell(pb) - last_pos); +seekback = FFMAX(seekback, 0); +avio_seek(pb, -seekback, SEEK_CUR); goto retry_read; } /* This is bad, we cannot find a valid frame header */ -- 2.1.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 5/8] ffmdec: break infinite resync loop
On Mon, Mar 09, 2015 at 03:14:50PM +0100, Andreas Cadhalpun wrote: > On 09.03.2015 13:59, Michael Niedermayer wrote: > >i was thinking more about limiting the backward seek before resync > >to the last resync position +1 if there was a previous resync > >so that resync which moves forward could not end before. This would > >avoid the failure and allow the demuxer to continue, or at least thats > >the idea > > I see, new patch attached. > The +1 is not necessary, because it always reads the PACKET_ID field > before resyncing. > > Best regards, > Andreas > > ffmdec.c |9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > 2f242fcf689ae8307a818b8a219d08af7c27921e > 0001-ffmdec-limit-the-backward-seek-to-the-last-resync-po.patch > From 6bcd1b4a906e2da39f6361db19826a337ac5a745 Mon Sep 17 00:00:00 2001 > From: Andreas Cadhalpun > Date: Mon, 9 Mar 2015 14:59:44 +0100 > Subject: [PATCH] ffmdec: limit the backward seek to the last resync position > > If resyncing leads to the same position as previously, it will again > lead to a resync attempt, resulting in an infinite loop. > > Thus don't seek back beyond the last syncpoint. applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Asymptotically faster algorithms should always be preferred if you have asymptotical amounts of data signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel