| Message ID | 20190812191708.22608-8-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
On Mon, Aug 12, 2019 at 9:20 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > Fixes: Timeout (14sec -> 133ms) > Fixes: > 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864 > Fixes: > 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280 > (35sec ->0.5sec) > > Why? This is bad idea, same like for qtrle and bunch of other cases. > 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> > --- > libavcodec/nuv.c | 37 ++++++++++++++++++++++++++++++++++--- > tests/ref/fate/nuv-rtjpeg | 1 - > 2 files changed, 34 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/nuv.c b/libavcodec/nuv.c > index 75b14bce5b..0952b537b7 100644 > --- a/libavcodec/nuv.c > +++ b/libavcodec/nuv.c > @@ -42,6 +42,8 @@ typedef struct NuvContext { > unsigned char *decomp_buf; > uint32_t lq[64], cq[64]; > RTJpegContext rtj; > + int need_flush; > + AVPacket flush_pkt; > } NuvContext; > > static const uint8_t fallback_lquant[] = { > @@ -66,6 +68,12 @@ static const uint8_t fallback_cquant[] = { > 99, 99, 99, 99, 99, 99, 99, 99 > }; > > +static void decode_flush(AVCodecContext *avctx){ > + NuvContext *s = avctx->priv_data; > + > + s->need_flush = 0; > +} > + > /** > * @brief copy frame data from buffer to AVFrame, handling stride. > * @param f destination AVFrame > @@ -172,6 +180,26 @@ static int decode_frame(AVCodecContext *avctx, void > *data, int *got_frame, > NUV_COPY_LAST = 'L' > } comptype; > > + if (!avpkt->data) { > + if (c->need_flush) { > + c->need_flush = 0; > + if ((ret = ff_reget_buffer(avctx, c->pic)) < 0) > + return ret; > + c->pic->pkt_pos = c->flush_pkt.pos; > + c->pic->pkt_duration = c->flush_pkt.duration; > + c->pic->pkt_dts = c->flush_pkt.dts; > + c->pic->pkt_pts = > + c->pic->pts = c->flush_pkt.pts; > + if ((ret = av_frame_ref(data, c->pic)) < 0) > + return ret; > + *got_frame = 1; > + } > + return 0; > + } > + c->flush_pkt = *avpkt; > + c->pic->pkt_dts = c->flush_pkt.dts; > + > + > if (buf_size < 12) { > av_log(avctx, AV_LOG_ERROR, "coded frame too small\n"); > return AVERROR_INVALIDDATA; > @@ -204,8 +232,8 @@ static int decode_frame(AVCodecContext *avctx, void > *data, int *got_frame, > } > break; > case NUV_COPY_LAST: > - keyframe = 0; > - break; > + c->need_flush = 1; > + return buf_size; > default: > keyframe = 1; > break; > @@ -313,6 +341,7 @@ retry: > if ((result = av_frame_ref(picture, c->pic)) < 0) > return result; > > + c->need_flush = 0; > *got_frame = 1; > return orig_size; > } > @@ -364,6 +393,8 @@ AVCodec ff_nuv_decoder = { > .init = decode_init, > .close = decode_end, > .decode = decode_frame, > - .capabilities = AV_CODEC_CAP_DR1, > + .flush = decode_flush, > + .caps_internal = FF_CODEC_CAP_SETS_PKT_DTS, > + .capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY, > .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > }; > diff --git a/tests/ref/fate/nuv-rtjpeg b/tests/ref/fate/nuv-rtjpeg > index b6f3b080dc..0914b985ec 100644 > --- a/tests/ref/fate/nuv-rtjpeg > +++ b/tests/ref/fate/nuv-rtjpeg > @@ -6,7 +6,6 @@ > 0, 118, 118, 0, 460800, 0x54aedafe > 0, 152, 152, 0, 460800, 0xb7aa8b56 > 0, 177, 177, 0, 460800, 0x283ea3b5 > -0, 202, 202, 0, 460800, 0x283ea3b5 > 0, 235, 235, 0, 460800, 0x10e577de > 0, 269, 269, 0, 460800, 0x4e091ee2 > 0, 302, 302, 0, 460800, 0x2ea88828 > -- > 2.22.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 Wed, Aug 14, 2019 at 06:02:13PM +0200, Paul B Mahol wrote: > On Mon, Aug 12, 2019 at 9:20 PM Michael Niedermayer <michael@niedermayer.cc> > wrote: > > > Fixes: Timeout (14sec -> 133ms) > > Fixes: > > 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864 > > Fixes: > > 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280 > > (35sec ->0.5sec) > > > > > Why? This is bad idea, same like for qtrle and bunch of other cases. This was discussed previously IIRC Either a codec can be used to turn tiny input to huge number of output frames by frame duplication. or it doesnt do the duplication and rather provides metadata of some sort In the first case if the next step is a filter it can be slower In the first case if the next step is a variable fps encoder it can be slower in the first case it can make a DOS attack less expensive for an attacker The second case, that is with metadata for example not returning a frame but relying on timestamps these issues are reduced thats why i suggest that way but if the community prefers something else then sure it can be done. But from what i remember the oppinions where mixed on which way is preferred thanks [...]
On 8/12/2019 4:17 PM, Michael Niedermayer wrote: > Fixes: Timeout (14sec -> 133ms) > Fixes: 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864 > Fixes: 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280 (35sec ->0.5sec) > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/nuv.c | 37 ++++++++++++++++++++++++++++++++++--- > tests/ref/fate/nuv-rtjpeg | 1 - > 2 files changed, 34 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/nuv.c b/libavcodec/nuv.c > index 75b14bce5b..0952b537b7 100644 > --- a/libavcodec/nuv.c > +++ b/libavcodec/nuv.c > @@ -42,6 +42,8 @@ typedef struct NuvContext { > unsigned char *decomp_buf; > uint32_t lq[64], cq[64]; > RTJpegContext rtj; > + int need_flush; > + AVPacket flush_pkt; > } NuvContext; > > static const uint8_t fallback_lquant[] = { > @@ -66,6 +68,12 @@ static const uint8_t fallback_cquant[] = { > 99, 99, 99, 99, 99, 99, 99, 99 > }; > > +static void decode_flush(AVCodecContext *avctx){ > + NuvContext *s = avctx->priv_data; > + > + s->need_flush = 0; > +} > + > /** > * @brief copy frame data from buffer to AVFrame, handling stride. > * @param f destination AVFrame > @@ -172,6 +180,26 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, > NUV_COPY_LAST = 'L' > } comptype; > > + if (!avpkt->data) { > + if (c->need_flush) { > + c->need_flush = 0; > + if ((ret = ff_reget_buffer(avctx, c->pic)) < 0) > + return ret; > + c->pic->pkt_pos = c->flush_pkt.pos; > + c->pic->pkt_duration = c->flush_pkt.duration; > + c->pic->pkt_dts = c->flush_pkt.dts; > + c->pic->pkt_pts = > + c->pic->pts = c->flush_pkt.pts; > + if ((ret = av_frame_ref(data, c->pic)) < 0) > + return ret; > + *got_frame = 1; Can't this be split off and shared in some way instead of implementing it for every decoder that needs to remove duplicate frames? Or maybe handle it in the generic decode.c code. > + } > + return 0; > + } > + c->flush_pkt = *avpkt; > + c->pic->pkt_dts = c->flush_pkt.dts; > + > + > if (buf_size < 12) { > av_log(avctx, AV_LOG_ERROR, "coded frame too small\n"); > return AVERROR_INVALIDDATA; > @@ -204,8 +232,8 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, > } > break; > case NUV_COPY_LAST: > - keyframe = 0; > - break; > + c->need_flush = 1; > + return buf_size; > default: > keyframe = 1; > break; > @@ -313,6 +341,7 @@ retry: > if ((result = av_frame_ref(picture, c->pic)) < 0) > return result; > > + c->need_flush = 0; > *got_frame = 1; > return orig_size; > } > @@ -364,6 +393,8 @@ AVCodec ff_nuv_decoder = { > .init = decode_init, > .close = decode_end, > .decode = decode_frame, > - .capabilities = AV_CODEC_CAP_DR1, > + .flush = decode_flush, > + .caps_internal = FF_CODEC_CAP_SETS_PKT_DTS, > + .capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY, > .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, > }; > diff --git a/tests/ref/fate/nuv-rtjpeg b/tests/ref/fate/nuv-rtjpeg > index b6f3b080dc..0914b985ec 100644 > --- a/tests/ref/fate/nuv-rtjpeg > +++ b/tests/ref/fate/nuv-rtjpeg > @@ -6,7 +6,6 @@ > 0, 118, 118, 0, 460800, 0x54aedafe > 0, 152, 152, 0, 460800, 0xb7aa8b56 > 0, 177, 177, 0, 460800, 0x283ea3b5 > -0, 202, 202, 0, 460800, 0x283ea3b5 > 0, 235, 235, 0, 460800, 0x10e577de > 0, 269, 269, 0, 460800, 0x4e091ee2 > 0, 302, 302, 0, 460800, 0x2ea88828 >
diff --git a/libavcodec/nuv.c b/libavcodec/nuv.c index 75b14bce5b..0952b537b7 100644 --- a/libavcodec/nuv.c +++ b/libavcodec/nuv.c @@ -42,6 +42,8 @@ typedef struct NuvContext { unsigned char *decomp_buf; uint32_t lq[64], cq[64]; RTJpegContext rtj; + int need_flush; + AVPacket flush_pkt; } NuvContext; static const uint8_t fallback_lquant[] = { @@ -66,6 +68,12 @@ static const uint8_t fallback_cquant[] = { 99, 99, 99, 99, 99, 99, 99, 99 }; +static void decode_flush(AVCodecContext *avctx){ + NuvContext *s = avctx->priv_data; + + s->need_flush = 0; +} + /** * @brief copy frame data from buffer to AVFrame, handling stride. * @param f destination AVFrame @@ -172,6 +180,26 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, NUV_COPY_LAST = 'L' } comptype; + if (!avpkt->data) { + if (c->need_flush) { + c->need_flush = 0; + if ((ret = ff_reget_buffer(avctx, c->pic)) < 0) + return ret; + c->pic->pkt_pos = c->flush_pkt.pos; + c->pic->pkt_duration = c->flush_pkt.duration; + c->pic->pkt_dts = c->flush_pkt.dts; + c->pic->pkt_pts = + c->pic->pts = c->flush_pkt.pts; + if ((ret = av_frame_ref(data, c->pic)) < 0) + return ret; + *got_frame = 1; + } + return 0; + } + c->flush_pkt = *avpkt; + c->pic->pkt_dts = c->flush_pkt.dts; + + if (buf_size < 12) { av_log(avctx, AV_LOG_ERROR, "coded frame too small\n"); return AVERROR_INVALIDDATA; @@ -204,8 +232,8 @@ static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, } break; case NUV_COPY_LAST: - keyframe = 0; - break; + c->need_flush = 1; + return buf_size; default: keyframe = 1; break; @@ -313,6 +341,7 @@ retry: if ((result = av_frame_ref(picture, c->pic)) < 0) return result; + c->need_flush = 0; *got_frame = 1; return orig_size; } @@ -364,6 +393,8 @@ AVCodec ff_nuv_decoder = { .init = decode_init, .close = decode_end, .decode = decode_frame, - .capabilities = AV_CODEC_CAP_DR1, + .flush = decode_flush, + .caps_internal = FF_CODEC_CAP_SETS_PKT_DTS, + .capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY, .caps_internal = FF_CODEC_CAP_INIT_CLEANUP, }; diff --git a/tests/ref/fate/nuv-rtjpeg b/tests/ref/fate/nuv-rtjpeg index b6f3b080dc..0914b985ec 100644 --- a/tests/ref/fate/nuv-rtjpeg +++ b/tests/ref/fate/nuv-rtjpeg @@ -6,7 +6,6 @@ 0, 118, 118, 0, 460800, 0x54aedafe 0, 152, 152, 0, 460800, 0xb7aa8b56 0, 177, 177, 0, 460800, 0x283ea3b5 -0, 202, 202, 0, 460800, 0x283ea3b5 0, 235, 235, 0, 460800, 0x10e577de 0, 269, 269, 0, 460800, 0x4e091ee2 0, 302, 302, 0, 460800, 0x2ea88828
Fixes: Timeout (14sec -> 133ms) Fixes: 14843/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5661969614372864 Fixes: 16257/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_NUV_fuzzer-5769175464673280 (35sec ->0.5sec) Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/nuv.c | 37 ++++++++++++++++++++++++++++++++++--- tests/ref/fate/nuv-rtjpeg | 1 - 2 files changed, 34 insertions(+), 4 deletions(-)