On Thu, Mar 09, 2017 at 07:59:37AM -0500, Ronald S. Bultje wrote: > Hi, > > On Wed, Mar 8, 2017 at 10:07 PM, Michael Niedermayer <mich...@niedermayer.cc > > wrote: > > > Fixes: timeout in 758/clusterfuzz-testcase-4720832028868608 > > > > Found-by: continuous fuzzing process https://github.com/google/oss- > > fuzz/tree/master/targets/ffmpeg > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > > --- > > libavcodec/vp56.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/libavcodec/vp56.c b/libavcodec/vp56.c > > index 0010408847..bccb424903 100644 > > --- a/libavcodec/vp56.c > > +++ b/libavcodec/vp56.c > > @@ -710,7 +710,7 @@ static int ff_vp56_decode_mbs(AVCodecContext *avctx, > > void *data, > > int ret = vp56_decode_mb(s, mb_row, mb_col, is_alpha); > > if (ret < 0) { > > damaged = 1; > > - if (!s->have_undamaged_frame) { > > + if (s->have_undamaged_frame < s->mb_width * > > s->mb_height) { > > s->discard_frame = 1; > > return AVERROR_INVALIDDATA; > > } > > @@ -732,7 +732,7 @@ static int ff_vp56_decode_mbs(AVCodecContext *avctx, > > void *data, > > } > > > > if (!damaged) > > - s->have_undamaged_frame = 1; > > + s->have_undamaged_frame = s->mb_width * s->mb_height; > > > You know very well that this makes the memory issue go away but isn't doing > the right thing if width1!=width2 && height1!=height2 but width1*height1 == > width2*height2. This is obviously because vpN codecs up to and including > vp8 don't include scalable MC. > > Can you do this right and only allow this if frame/ref width and height > both match, not just their product?
you assume that there is a out of array access and this is fixing it and argue that this is the wrong fix for it. You are correct that this would be the wrong fix if that was the bug. Its possible there is such a bug, but i have not seen that. What this is about is a timeout, as described in the commit message a small file with a tiny initial frame followed by frames with huge resolution but very few bits the code is timing out as the error concealment takes alot of time for high resolutions, no memory anomalies occured here when the concealment code runs so any reference frame must be large enough. The solution this patch implements is to require at least a undamagd frame with X MBs before allowing concealment of similarly high resolution frames. This undamagd frame is quite possibly not used by the concealment its rather a check to make sure the file is not just "empty" This directly combats the issue of crafted files that are very small bytewise but take a huge amount of time to decode. Its quite possible the fuzzer will find a hole in this and we may require to count undamaged mbs over several frames to weed out these slow to decode empty timeout cases, we will only know once the easier case is closed [...] -- 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