diff mbox series

[FFmpeg-devel,2/7] avcodec/vc1dec: Postpone allocating sprite frame to avoid segfault

Message ID 20201225154724.287465-2-andreas.rheinhardt@gmail.com
State Accepted
Commit ea70c39deecb64f94fe7ae0ea419cbef1c7f9372
Headers show
Series [FFmpeg-devel,1/7] avcodec/mpeg12dec: Remove update_thread_context | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Dec. 25, 2020, 3:47 p.m. UTC
Up until now, the VC-1 decoders allocated an AVFrame for usage with
sprites during vc1_decode_init(); yet said AVFrame can be freed if
(re)initializing the context (which happens ordinarily during decoding)
fails. The AVFrame does not get allocated again lateron in this case,
leading to segfaults.

Fix this by moving the allocation of said frame immediately before it is
used (this also means that said frame won't be allocated at all any more
in case of a regular (i.e. non-image) stream).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/vc1dec.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

Comments

Andreas Rheinhardt March 24, 2021, 11:02 a.m. UTC | #1
Andreas Rheinhardt:
> Up until now, the VC-1 decoders allocated an AVFrame for usage with
> sprites during vc1_decode_init(); yet said AVFrame can be freed if
> (re)initializing the context (which happens ordinarily during decoding)
> fails. The AVFrame does not get allocated again lateron in this case,
> leading to segfaults.
> 
> Fix this by moving the allocation of said frame immediately before it is
> used (this also means that said frame won't be allocated at all any more
> in case of a regular (i.e. non-image) stream).
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/vc1dec.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
> index 7809234ff7..5cdf197da7 100644
> --- a/libavcodec/vc1dec.c
> +++ b/libavcodec/vc1dec.c
> @@ -539,12 +539,6 @@ static av_cold int vc1_decode_init(AVCodecContext *avctx)
>      ff_h264chroma_init(&v->h264chroma, 8);
>      ff_qpeldsp_init(&s->qdsp);
>  
> -    // Must happen after calling ff_vc1_decode_end
> -    // to avoid de-allocating the sprite_output_frame
> -    v->sprite_output_frame = av_frame_alloc();
> -    if (!v->sprite_output_frame)
> -        return AVERROR(ENOMEM);
> -
>      avctx->has_b_frames = !!avctx->max_b_frames;
>  
>      if (v->color_prim == 1 || v->color_prim == 5 || v->color_prim == 6)
> @@ -577,20 +571,15 @@ static av_cold int vc1_decode_init(AVCodecContext *avctx)
>              v->sprite_height > 1 << 14 ||
>              v->output_width  > 1 << 14 ||
>              v->output_height > 1 << 14) {
> -            ret = AVERROR_INVALIDDATA;
> -            goto error;
> +            return AVERROR_INVALIDDATA;
>          }
>  
>          if ((v->sprite_width&1) || (v->sprite_height&1)) {
>              avpriv_request_sample(avctx, "odd sprites support");
> -            ret = AVERROR_PATCHWELCOME;
> -            goto error;
> +            return AVERROR_PATCHWELCOME;
>          }
>      }
>      return 0;
> -error:
> -    av_frame_free(&v->sprite_output_frame);
> -    return ret;
>  }
>  
>  /** Close a VC1/WMV3 decoder
> @@ -1147,6 +1136,11 @@ image:
>          avctx->height = avctx->coded_height = v->output_height;
>          if (avctx->skip_frame >= AVDISCARD_NONREF)
>              goto end;
> +        if (!v->sprite_output_frame &&
> +            !(v->sprite_output_frame = av_frame_alloc())) {
> +            ret = AVERROR(ENOMEM);
> +            goto err;
> +        }
>  #if CONFIG_WMV3IMAGE_DECODER || CONFIG_VC1IMAGE_DECODER
>          if ((ret = vc1_decode_sprites(v, &s->gb)) < 0)
>              goto err;
> 
Will apply tonight unless there are objections.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
index 7809234ff7..5cdf197da7 100644
--- a/libavcodec/vc1dec.c
+++ b/libavcodec/vc1dec.c
@@ -539,12 +539,6 @@  static av_cold int vc1_decode_init(AVCodecContext *avctx)
     ff_h264chroma_init(&v->h264chroma, 8);
     ff_qpeldsp_init(&s->qdsp);
 
-    // Must happen after calling ff_vc1_decode_end
-    // to avoid de-allocating the sprite_output_frame
-    v->sprite_output_frame = av_frame_alloc();
-    if (!v->sprite_output_frame)
-        return AVERROR(ENOMEM);
-
     avctx->has_b_frames = !!avctx->max_b_frames;
 
     if (v->color_prim == 1 || v->color_prim == 5 || v->color_prim == 6)
@@ -577,20 +571,15 @@  static av_cold int vc1_decode_init(AVCodecContext *avctx)
             v->sprite_height > 1 << 14 ||
             v->output_width  > 1 << 14 ||
             v->output_height > 1 << 14) {
-            ret = AVERROR_INVALIDDATA;
-            goto error;
+            return AVERROR_INVALIDDATA;
         }
 
         if ((v->sprite_width&1) || (v->sprite_height&1)) {
             avpriv_request_sample(avctx, "odd sprites support");
-            ret = AVERROR_PATCHWELCOME;
-            goto error;
+            return AVERROR_PATCHWELCOME;
         }
     }
     return 0;
-error:
-    av_frame_free(&v->sprite_output_frame);
-    return ret;
 }
 
 /** Close a VC1/WMV3 decoder
@@ -1147,6 +1136,11 @@  image:
         avctx->height = avctx->coded_height = v->output_height;
         if (avctx->skip_frame >= AVDISCARD_NONREF)
             goto end;
+        if (!v->sprite_output_frame &&
+            !(v->sprite_output_frame = av_frame_alloc())) {
+            ret = AVERROR(ENOMEM);
+            goto err;
+        }
 #if CONFIG_WMV3IMAGE_DECODER || CONFIG_VC1IMAGE_DECODER
         if ((ret = vc1_decode_sprites(v, &s->gb)) < 0)
             goto err;