[FFmpeg-devel] [PATCH] avcodec/mlz: Add the check after calling av_mallocz

[FFmpeg-devel] [PATCH] avcodec/mlz: Add the check after calling av_mallocz

[Jiasheng Jiang]

Since the potential failure of memory allocation, the av_mallocz()
may return NULL pointer if fails, which is assigned to 'mlz->dict'.
And then 'mlz->dict' will be used in ff_mlz_flush_dict().
Therefore, it should be better to check it and return error if fails
in order to prevent the dereference of the NULL pointer.
Also, the caller, the decode_init() needs to deal with the return value
of ff_mlz_init_dict().

Fixes: 2f7a12fab5 ("avcodec/mlz: clear dict on allocation to ensure there are no uninitialized values")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 libavcodec/alsdec.c | 5 ++++-
 libavcodec/mlz.c    | 6 +++++-
 libavcodec/mlz.h    | 2 +-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/libavcodec/alsdec.c b/libavcodec/alsdec.c
index 9e1aaf065a..2fbb309d33 100644
--- a/libavcodec/alsdec.c
+++ b/libavcodec/alsdec.c
@@ -2122,7 +2122,10 @@ static av_cold int decode_init(AVCodecContext *avctx)
             goto fail;
         }
 
-        ff_mlz_init_dict(avctx, ctx->mlz);
+        ret = ff_mlz_init_dict(avctx, ctx->mlz);
+        if (ret < 0)
+            goto fail;
+
         ff_mlz_flush_dict(ctx->mlz);
 
         for (c = 0; c < avctx->channels; ++c) {
diff --git a/libavcodec/mlz.c b/libavcodec/mlz.c
index dbeb7dcad9..b35607cc7c 100644
--- a/libavcodec/mlz.c
+++ b/libavcodec/mlz.c
@@ -20,8 +20,10 @@
 
 #include "mlz.h"
 
-av_cold void ff_mlz_init_dict(void* context, MLZ *mlz) {
+av_cold int ff_mlz_init_dict(void* context, MLZ *mlz) {
     mlz->dict = av_mallocz(TABLE_SIZE * sizeof(*mlz->dict));
+    if (!mlz->dict)
+        return AVERROR(ENOMEM);
 
     mlz->flush_code            = FLUSH_CODE;
     mlz->current_dic_index_max = DIC_INDEX_INIT;
@@ -30,6 +32,8 @@ av_cold void ff_mlz_init_dict(void* context, MLZ *mlz) {
     mlz->next_code             = FIRST_CODE;
     mlz->freeze_flag           = 0;
     mlz->context               = context;
+
+    return 0;
 }
 
 av_cold void ff_mlz_flush_dict(MLZ *mlz) {
diff --git a/libavcodec/mlz.h b/libavcodec/mlz.h
index c3df52c9b4..01f8e78ec2 100644
--- a/libavcodec/mlz.h
+++ b/libavcodec/mlz.h
@@ -57,7 +57,7 @@ typedef struct MLZ {
 
 /** Initialize the dictionary
  */
-void ff_mlz_init_dict(void* context, MLZ *mlz);
+int ff_mlz_init_dict(void* context, MLZ *mlz);
 
 /** Flush the dictionary
  */
-- 
2.25.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] avcodec/mlz: Add the check after calling av_mallocz

[Andreas Rheinhardt]

Jiasheng Jiang:
> Since the potential failure of memory allocation, the av_mallocz()
> may return NULL pointer if fails, which is assigned to 'mlz->dict'.
> And then 'mlz->dict' will be used in ff_mlz_flush_dict().
> Therefore, it should be better to check it and return error if fails
> in order to prevent the dereference of the NULL pointer.
> Also, the caller, the decode_init() needs to deal with the return value
> of ff_mlz_init_dict().
> 
> Fixes: 2f7a12fab5 ("avcodec/mlz: clear dict on allocation to ensure there are no uninitialized values")

This is the wrong reference; there was an unchecked allocation before
that, it was just and unchecked av_malloc_array instead of av_mallocz_array.

> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> ---
>  libavcodec/alsdec.c | 5 ++++-
>  libavcodec/mlz.c    | 6 +++++-
>  libavcodec/mlz.h    | 2 +-
>  3 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/alsdec.c b/libavcodec/alsdec.c
> index 9e1aaf065a..2fbb309d33 100644
> --- a/libavcodec/alsdec.c
> +++ b/libavcodec/alsdec.c
> @@ -2122,7 +2122,10 @@ static av_cold int decode_init(AVCodecContext *avctx)
>              goto fail;
>          }
>  
> -        ff_mlz_init_dict(avctx, ctx->mlz);
> +        ret = ff_mlz_init_dict(avctx, ctx->mlz);
> +        if (ret < 0)
> +            goto fail;
> +
>          ff_mlz_flush_dict(ctx->mlz);
>  
>          for (c = 0; c < avctx->channels; ++c) {
> diff --git a/libavcodec/mlz.c b/libavcodec/mlz.c
> index dbeb7dcad9..b35607cc7c 100644
> --- a/libavcodec/mlz.c
> +++ b/libavcodec/mlz.c
> @@ -20,8 +20,10 @@
>  
>  #include "mlz.h"
>  
> -av_cold void ff_mlz_init_dict(void* context, MLZ *mlz) {
> +av_cold int ff_mlz_init_dict(void* context, MLZ *mlz) {
>      mlz->dict = av_mallocz(TABLE_SIZE * sizeof(*mlz->dict));
> +    if (!mlz->dict)
> +        return AVERROR(ENOMEM);
>  
>      mlz->flush_code            = FLUSH_CODE;
>      mlz->current_dic_index_max = DIC_INDEX_INIT;
> @@ -30,6 +32,8 @@ av_cold void ff_mlz_init_dict(void* context, MLZ *mlz) {
>      mlz->next_code             = FIRST_CODE;
>      mlz->freeze_flag           = 0;
>      mlz->context               = context;
> +
> +    return 0;
>  }
>  
>  av_cold void ff_mlz_flush_dict(MLZ *mlz) {
> diff --git a/libavcodec/mlz.h b/libavcodec/mlz.h
> index c3df52c9b4..01f8e78ec2 100644
> --- a/libavcodec/mlz.h
> +++ b/libavcodec/mlz.h
> @@ -57,7 +57,7 @@ typedef struct MLZ {
>  
>  /** Initialize the dictionary
>   */
> -void ff_mlz_init_dict(void* context, MLZ *mlz);
> +int ff_mlz_init_dict(void* context, MLZ *mlz);
>  
>  /** Flush the dictionary
>   */

See https://ffmpeg.org/pipermail/ffmpeg-devel/2022-February/292904.html

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