On 9/29/2020 12:57 PM, Mark Thompson wrote: > On 25/09/2020 15:43, James Almer wrote: >> Fixes: member access within null pointer of type 'TileGroupInfo' (aka >> 'struct TileGroupInfo') >> Fixes: >> 25725/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AV1_fuzzer-5166692706287616 >> >> >> Found-by: continuous fuzzing process >> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >> Signed-off-by: James Almer <[email protected]> >> --- >> libavcodec/av1dec.c | 30 ++++++++++++++++-------------- >> 1 file changed, 16 insertions(+), 14 deletions(-) >> >> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c >> index 07026b7aeb..e5cfc3f2f2 100644 >> --- a/libavcodec/av1dec.c >> +++ b/libavcodec/av1dec.c >> @@ -381,6 +381,20 @@ fail: >> return AVERROR(ENOMEM); >> } >> +static void av1_decode_flush(AVCodecContext *avctx) >> +{ >> + AV1DecContext *s = avctx->priv_data; >> + >> + for (int i = 0; i < FF_ARRAY_ELEMS(s->ref); i++) >> + av1_frame_unref(avctx, &s->ref[i]); >> + >> + av1_frame_unref(avctx, &s->cur_frame); >> + s->raw_frame_header = NULL; >> + s->raw_seq = NULL; >> + >> + ff_cbs_flush(s->cbc); >> +} >> + >> static av_cold int av1_decode_free(AVCodecContext *avctx) >> { >> AV1DecContext *s = avctx->priv_data; >> @@ -841,23 +855,11 @@ static int av1_decode_frame(AVCodecContext >> *avctx, void *frame, >> end: >> ff_cbs_fragment_reset(&s->current_obu); >> + if (ret < 0) >> + av1_decode_flush(avctx); >> return ret; >> } >> -static void av1_decode_flush(AVCodecContext *avctx) >> -{ >> - AV1DecContext *s = avctx->priv_data; >> - >> - for (int i = 0; i < FF_ARRAY_ELEMS(s->ref); i++) >> - av1_frame_unref(avctx, &s->ref[i]); >> - >> - av1_frame_unref(avctx, &s->cur_frame); >> - s->raw_frame_header = NULL; >> - s->raw_seq = NULL; >> - >> - ff_cbs_flush(s->cbc); >> -} >> - >> AVCodec ff_av1_decoder = { >> .name = "av1", >> .long_name = NULL_IF_CONFIG_SMALL("Alliance for Open >> Media AV1"), >> > > This seems questionable - if you have some packet loss and lose an > intermediate frame, I don't think you want to throw away all the old > state since it may be able to continue from an earlier frame which was > received correctly.
If a frame was not parsed correctly, then the reference frame state from then on will be invalid. Especially if ff_cbs_read_packet() is what failed, considering everything after the corrupt OBU within the TU will be discarded by CBS. I can replace this to simply setting raw_frame_header to NULL if you prefer, but for reference, dav1d (the decoder that hopefully will eventually be ported into this native decoder) throws the entire state away on a frame decoding failure. > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > [email protected] > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > [email protected] with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
