| Message ID | 20190821221855.1844-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | 493438fafc5c43b7b7c62bf0c21b7cc884034ce9 |
| Headers | show |
On 8/21/2019 7:18 PM, Michael Niedermayer wrote: > Fixes: memleak > Fixes: 16277/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5696629440512000 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/realtextdec.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavformat/realtextdec.c b/libavformat/realtextdec.c > index 204e557aa2..c2316da0ed 100644 > --- a/libavformat/realtextdec.c > +++ b/libavformat/realtextdec.c > @@ -123,6 +123,8 @@ static int realtext_read_header(AVFormatContext *s) > > end: > av_bprint_finalize(&buf, NULL); > + if (res < 0) > + ff_subtitles_queue_clean(&rt->q); LGTM > return res; > } > >
Why not fix issues listed in decoder code instead of this hack? On Thu, Aug 22, 2019 at 12:21 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > Fixes: Timeout (128sec -> 6sec) > Fixes: > 16568/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_IDCIN_fuzzer-5675004095627264 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by > <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>: > Michael Niedermayer <michael@niedermayer.cc> > --- > tools/target_dec_fuzzer.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tools/target_dec_fuzzer.c b/tools/target_dec_fuzzer.c > index d83039417c..fd76553ec8 100644 > --- a/tools/target_dec_fuzzer.c > +++ b/tools/target_dec_fuzzer.c > @@ -177,6 +177,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t > size) { > case AV_CODEC_ID_GDV: maxpixels /= 256; break; > // Postprocessing in C > case AV_CODEC_ID_HNM4_VIDEO:maxpixels /= 128; break; > + // Unoptimized VLC reading and allows a small input to generate > gigantic output > + case AV_CODEC_ID_IDCIN: maxpixels /= 2048; break; > } > > > -- > 2.23.0 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Thu, Aug 22, 2019 at 10:09:30AM +0200, Paul B Mahol wrote: > Why not fix issues listed in decoder code instead of this hack? I can improve the VLC reader and will probably look into that but i dont think that will provide the factor of 2000 speedup needed to avoid the adjustment. It might reduce the factor needed though. thx [...] > > + // Unoptimized VLC reading and allows a small input to generate > > gigantic output > > + case AV_CODEC_ID_IDCIN: maxpixels /= 2048; break; [...]
On Thu, Aug 22, 2019 at 10:16 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Thu, Aug 22, 2019 at 10:09:30AM +0200, Paul B Mahol wrote: > > Why not fix issues listed in decoder code instead of this hack? > > I can improve the VLC reader and will probably look into that > but i dont think that will provide the factor of 2000 speedup needed > to avoid the adjustment. It might reduce the factor needed though. > > Also all this old game codecs never used very high resolutions. Max something vga, but not even that. > thx > > > [...] > > > > + // Unoptimized VLC reading and allows a small input to > generate > > > gigantic output > > > + case AV_CODEC_ID_IDCIN: maxpixels /= 2048; break; > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Take away the freedom of one citizen and you will be jailed, take away > the freedom of all citizens and you will be congratulated by your peers > in Parliament. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Thu, Aug 22, 2019 at 10:23:40AM +0200, Paul B Mahol wrote: > On Thu, Aug 22, 2019 at 10:16 AM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > On Thu, Aug 22, 2019 at 10:09:30AM +0200, Paul B Mahol wrote: > > > Why not fix issues listed in decoder code instead of this hack? > > > > I can improve the VLC reader and will probably look into that > > but i dont think that will provide the factor of 2000 speedup needed > > to avoid the adjustment. It might reduce the factor needed though. > > > > > Also all this old game codecs never used very high resolutions. > Max something vga, but not even that. limiting to 640x480 instead of 4096x4096 would give a factor of about 55 speedup, thats still a long way from 2000 so that solves also only part of it anyway ill provide a patch that limits resolution so theres something that can be discussed. Thanks [...]
tor 2019-08-22 klockan 10:34 +0200 skrev Michael Niedermayer: > On Thu, Aug 22, 2019 at 10:23:40AM +0200, Paul B Mahol wrote: > > On Thu, Aug 22, 2019 at 10:16 AM Michael Niedermayer < > > michael@niedermayer.cc> > > wrote: > > > > > On Thu, Aug 22, 2019 at 10:09:30AM +0200, Paul B Mahol wrote: > > > > Why not fix issues listed in decoder code instead of this hack? I think there's better things we could have Michael do. That said: > > Also all this old game codecs never used very high resolutions. > > Max something vga, but not even that. > > limiting to 640x480 instead of 4096x4096 would give a factor of about > 55 > speedup, thats still a long way from 2000 so that solves also only > part of it > > anyway ill provide a patch that limits resolution so theres something > that can be discussed. The probing in lavf/idcin.c already limits the resolution to 1024x1024. The only sample we have is 320x240. The Quake 2 decoder (cl_cin.c) limits CIN frames to 128 KiB compressed, from which the 1024x1024 limit in idcinvideo.c likely derives (128 KiB == 1 Mib == 1 Mipx if always using the shortest Huffman code). Zero-byte packets are also not allowed. If we're looking over this anyway we could toss in all these limitations, and maybe limit audio to 11025, 22050 and 44100 Hz. I also notice that idcinvideo.c contains code copy-pasted from cl_cin.c in Quake 2 (SmallestNode1 -> huff_smallest_node). The copyright header should reflect this. /Tomas
On Thu, Aug 22, 2019 at 11:24 AM Tomas Härdin <tjoppen@acc.umu.se> wrote: > tor 2019-08-22 klockan 10:34 +0200 skrev Michael Niedermayer: > > On Thu, Aug 22, 2019 at 10:23:40AM +0200, Paul B Mahol wrote: > > > On Thu, Aug 22, 2019 at 10:16 AM Michael Niedermayer < > > > michael@niedermayer.cc> > > > wrote: > > > > > > > On Thu, Aug 22, 2019 at 10:09:30AM +0200, Paul B Mahol wrote: > > > > > Why not fix issues listed in decoder code instead of this hack? > > I think there's better things we could have Michael do. That said: > Yes, more timeouts fixes as always. > > > > Also all this old game codecs never used very high resolutions. > > > Max something vga, but not even that. > > > > limiting to 640x480 instead of 4096x4096 would give a factor of about > > 55 > > speedup, thats still a long way from 2000 so that solves also only > > part of it > > > > anyway ill provide a patch that limits resolution so theres something > > that can be discussed. > > The probing in lavf/idcin.c already limits the resolution to 1024x1024. > The only sample we have is 320x240. The Quake 2 decoder (cl_cin.c) > limits CIN frames to 128 KiB compressed, from which the 1024x1024 limit > in idcinvideo.c likely derives (128 KiB == 1 Mib == 1 Mipx if always > using the shortest Huffman code). Zero-byte packets are also not > allowed. > > If we're looking over this anyway we could toss in all these > limitations, and maybe limit audio to 11025, 22050 and 44100 Hz. > > I also notice that idcinvideo.c contains code copy-pasted from cl_cin.c > in Quake 2 (SmallestNode1 -> huff_smallest_node). The copyright header > should reflect this. > That is sole reason why that awful code should not be kept, and instead should be rewritten. > /Tomas > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Wed, Aug 21, 2019 at 08:48:25PM -0300, James Almer wrote: > On 8/21/2019 7:18 PM, Michael Niedermayer wrote: > > Fixes: memleak > > Fixes: 16277/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5696629440512000 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/realtextdec.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/libavformat/realtextdec.c b/libavformat/realtextdec.c > > index 204e557aa2..c2316da0ed 100644 > > --- a/libavformat/realtextdec.c > > +++ b/libavformat/realtextdec.c > > @@ -123,6 +123,8 @@ static int realtext_read_header(AVFormatContext *s) > > > > end: > > av_bprint_finalize(&buf, NULL); > > + if (res < 0) > > + ff_subtitles_queue_clean(&rt->q); > > LGTM will apply thx [...]
diff --git a/libavformat/realtextdec.c b/libavformat/realtextdec.c index 204e557aa2..c2316da0ed 100644 --- a/libavformat/realtextdec.c +++ b/libavformat/realtextdec.c @@ -123,6 +123,8 @@ static int realtext_read_header(AVFormatContext *s) end: av_bprint_finalize(&buf, NULL); + if (res < 0) + ff_subtitles_queue_clean(&rt->q); return res; }
Fixes: memleak Fixes: 16277/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5696629440512000 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/realtextdec.c | 2 ++ 1 file changed, 2 insertions(+)