[FFmpeg-devel] [PATCH 8/8] avcodec/av1dec: clean state on frame decoding errors

[Mark Thompson]

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 <jamrial at gmail.com>
> ---
>   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.

- Mark