sön 2019-08-18 klockan 02:35 +0200 skrev Tomas Härdin:
> lör 2019-08-17 klockan 17:33 +0200 skrev Michael Niedermayer:
> > On Sat, Aug 17, 2019 at 12:26:27AM +0200, Tomas Härdin wrote:
> > > fre 2019-08-16 klockan 14:57 +0200 skrev Tomas Härdin:
> > >
> > > I feel I should point out that being conservative here is at odds with
> > > the general "best effort" approach taken in this project. These toy
> > > codecs are useful as illustrative examples of this contradiction. I'm
> > > sure there are many more examples of files that can cause ffmpeg to do
> > > a lot more work than expected, for almost every codec. I know afl-fuzz
> > > is likely to find out that it can make the ZMBV decoder do a lot of
> > > work for a small input file, because I've seen it do that with gzip.
> > >
> > > The user base for cinepak is of course miniscule, so I doubt anyone's
> > > going to complain that their broken CVID files don't work any more. I
> > > certainly don't care since cinepakenc only puts out valid files.
> > > But
> > > again, for other formats we're just going to have to tell users to put
> > > ffmpeg inside a sandbox and stick a CPU limit on it. Even ffprobe is
> > > vulnerable to DoS-y things.
> >
> > yes
> >
> > the question ATM is just what to do here about this codec ?
> > apply the patch ?
> > change it ?
>
> Well for a start, the file is 65535 x 209 pixels, 3166 frames. I
> wouldn't call decoding that @ 263 fps particularly slow
>
> Second, it's not the decoder which is slow. If I comment out the
> "*got_frame = 1;" then the test also runs fast. I'm not sure what
> happens elsewhere with the decoded buffer, but I suspect there's a
> bunch of useless malloc()/memset()ing going on. Maybe the decoder is
> using ff_reget_buffer() or av_frame_ref() incorrectly, I'm not sure.
I did some investigation, it is indeed ff_reget_buffer(). It copies the
frame data for some reason. The fix is simple in this case: just call
ff_get_buffer() once in cinepak_decode_init() and keep overwriting the
same frame.
> As I said on IRC, this class of problems will exist for every codec.
> Cinepak is easy to decode, even at these resolutions. Just imagine what
> will happens when someone feeds in a 65535x209 av1 stream..
And related to this, ff_reget_buffer() is used for a lot of these
codecs which only overwrite pixels in the old frame. flicvideo, gifdec,
msrle, roqvideodec and others probably have the same flaw.
Patched attached.
/Tomas
From c85f23ca4c42f9ce27f512be897214b8689c1c94 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <tjop...@acc.umu.se>
Date: Sun, 18 Aug 2019 10:30:59 +0200
Subject: [PATCH] libavcodec/cinepak: Avoid copying frame data
We can just keep overwriting the same frame.
This speeds up the decoder, especially for very
large frames, since skip MBs are turned into nops.
clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_CINEPAK_fuzzer-5695832885559296
32577 ms -> 42 ms
---
libavcodec/cinepak.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/libavcodec/cinepak.c b/libavcodec/cinepak.c
index aeb15de0ed..e3e6ecfdf0 100644
--- a/libavcodec/cinepak.c
+++ b/libavcodec/cinepak.c
@@ -424,6 +424,7 @@ static int cinepak_decode (CinepakContext *s)
static av_cold int cinepak_decode_init(AVCodecContext *avctx)
{
CinepakContext *s = avctx->priv_data;
+ int ret;
s->avctx = avctx;
s->width = (avctx->width + 3) & ~3;
@@ -444,6 +445,9 @@ static av_cold int cinepak_decode_init(AVCodecContext *avctx)
if (!s->frame)
return AVERROR(ENOMEM);
+ if ((ret = ff_get_buffer(avctx, s->frame, 0)) < 0)
+ return ret;
+
return 0;
}
@@ -473,9 +477,6 @@ static int cinepak_decode_frame(AVCodecContext *avctx,
return ret;
}
- if ((ret = ff_reget_buffer(avctx, s->frame)) < 0)
- return ret;
-
if (s->palette_video) {
int size;
const uint8_t *pal = av_packet_get_side_data(avpkt, AV_PKT_DATA_PALETTE, &size);
--
2.20.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
