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

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

[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;
 
-- 
2.45.1

_______________________________________________
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".

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

[Andreas Rheinhardt]

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

_______________________________________________
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".