diff mbox series

[FFmpeg-devel] lavc/vvc: Don't free uninitialised pic arrays

Message ID 20240531133625.98622-1-post@frankplowman.com
State New
Headers show
Series [FFmpeg-devel] lavc/vvc: Don't free uninitialised pic arrays | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Frank Plowman May 31, 2024, 1:36 p.m. UTC
The picture arrays are not initialised at the same time as the frame
context itself, but rather when the relevant frame begins being decoded.
As such, situations can arise where the frame context is being freed but
the picture arrays have not yet been initialised.  This could lead to
various UB and ultimately crashes.  Patch prevents this by adding an
initialised flag associated with the picture arrays.

Signed-off-by: Frank Plowman <post@frankplowman.com>
---
 libavcodec/vvc/dec.c | 7 +++++++
 libavcodec/vvc/dec.h | 2 ++
 2 files changed, 9 insertions(+)

Comments

Andreas Rheinhardt May 31, 2024, 4:11 p.m. UTC | #1
Frank Plowman:
> The picture arrays are not initialised at the same time as the frame
> context itself, but rather when the relevant frame begins being decoded.
> As such, situations can arise where the frame context is being freed but
> the picture arrays have not yet been initialised.  This could lead to
> various UB and ultimately crashes.  Patch prevents this by adding an
> initialised flag associated with the picture arrays.
> 
> Signed-off-by: Frank Plowman <post@frankplowman.com>
> ---
>  libavcodec/vvc/dec.c | 7 +++++++
>  libavcodec/vvc/dec.h | 2 ++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
> index e53ad4e607..32e5bc0cd8 100644
> --- a/libavcodec/vvc/dec.c
> +++ b/libavcodec/vvc/dec.c
> @@ -327,6 +327,9 @@ static void free_cus(VVCFrameContext *fc)
>  
>  static void pic_arrays_free(VVCFrameContext *fc)
>  {
> +    if (!fc->tab.initialised)
> +        return;
> +
>      free_cus(fc);
>      frame_context_for_each_tl(fc, tl_free);
>      ff_refstruct_pool_uninit(&fc->rpl_tab_pool);
> @@ -380,6 +383,8 @@ static int pic_arrays_init(VVCContext *s, VVCFrameContext *fc)
>      fc->tab.sz.bs_width           = (fc->ps.pps->width >> 2) + 1;
>      fc->tab.sz.bs_height          = (fc->ps.pps->height >> 2) + 1;
>  
> +    fc->tab.initialised = 1;
> +
>      return 0;
>  }
>  
> @@ -627,6 +632,8 @@ static av_cold int frame_context_init(VVCFrameContext *fc, AVCodecContext *avctx
>      if (!fc->tu_pool)
>          return AVERROR(ENOMEM);
>  
> +    fc->tab.initialised = 0;
> +
>      return 0;
>  }
>  
> diff --git a/libavcodec/vvc/dec.h b/libavcodec/vvc/dec.h
> index 1e0b76f283..1721ba3a15 100644
> --- a/libavcodec/vvc/dec.h
> +++ b/libavcodec/vvc/dec.h
> @@ -212,6 +212,8 @@ typedef struct VVCFrameContext {
>              int bs_height;
>              int ibc_buffer_width;       ///< IbcBufWidth
>          } sz;
> +
> +        int initialised;
>      } tab;
>  } VVCFrameContext;
>  

This will lead to leaks when an error happens in pic_arrays_init() after
some allocations succeeded.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/vvc/dec.c b/libavcodec/vvc/dec.c
index e53ad4e607..32e5bc0cd8 100644
--- a/libavcodec/vvc/dec.c
+++ b/libavcodec/vvc/dec.c
@@ -327,6 +327,9 @@  static void free_cus(VVCFrameContext *fc)
 
 static void pic_arrays_free(VVCFrameContext *fc)
 {
+    if (!fc->tab.initialised)
+        return;
+
     free_cus(fc);
     frame_context_for_each_tl(fc, tl_free);
     ff_refstruct_pool_uninit(&fc->rpl_tab_pool);
@@ -380,6 +383,8 @@  static int pic_arrays_init(VVCContext *s, VVCFrameContext *fc)
     fc->tab.sz.bs_width           = (fc->ps.pps->width >> 2) + 1;
     fc->tab.sz.bs_height          = (fc->ps.pps->height >> 2) + 1;
 
+    fc->tab.initialised = 1;
+
     return 0;
 }
 
@@ -627,6 +632,8 @@  static av_cold int frame_context_init(VVCFrameContext *fc, AVCodecContext *avctx
     if (!fc->tu_pool)
         return AVERROR(ENOMEM);
 
+    fc->tab.initialised = 0;
+
     return 0;
 }
 
diff --git a/libavcodec/vvc/dec.h b/libavcodec/vvc/dec.h
index 1e0b76f283..1721ba3a15 100644
--- a/libavcodec/vvc/dec.h
+++ b/libavcodec/vvc/dec.h
@@ -212,6 +212,8 @@  typedef struct VVCFrameContext {
             int bs_height;
             int ibc_buffer_width;       ///< IbcBufWidth
         } sz;
+
+        int initialised;
     } tab;
 } VVCFrameContext;