diff mbox series

[FFmpeg-devel,2/4] avcodec/vp3: Check allocations of VLCs

Message ID 20201020075356.185676-2-andreas.rheinhardt@gmail.com
State Accepted
Commit 786b1b0c44d3e5d71c3e69a3fe260baa95172e02
Headers show
Series [FFmpeg-devel,1/4] avcodec/vp3: Fix memleak upon init failure
Related show

Checks

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

Commit Message

Andreas Rheinhardt Oct. 20, 2020, 7:53 a.m. UTC
It would lead to crashs lateron if they failed.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/vp3.c | 112 +++++++++++++++++++++++++++--------------------
 1 file changed, 64 insertions(+), 48 deletions(-)

Comments

Peter Ross Oct. 20, 2020, 9:28 p.m. UTC | #1
On Tue, Oct 20, 2020 at 09:53:53AM +0200, Andreas Rheinhardt wrote:
> It would lead to crashs lateron if they failed.

did you find these automatically?

have previously used this tool tool to find such errors: https://github.com/ralight/mallocfail

> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavcodec/vp3.c | 112 +++++++++++++++++++++++++++--------------------
>  1 file changed, 64 insertions(+), 48 deletions(-)
> 
> diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c
> index 0d8d1ff80f..7ee65c8062 100644
> --- a/libavcodec/vp3.c
> +++ b/libavcodec/vp3.c
> @@ -2414,57 +2414,67 @@ static av_cold int vp3_decode_init(AVCodecContext *avctx)
>          if (s->version < 2) {
>              for (i = 0; i < 16; i++) {
>                  /* DC histograms */
> -                init_vlc(&s->dc_vlc[i], 11, 32,
> -                         &dc_bias[i][0][1], 4, 2,
> -                         &dc_bias[i][0][0], 4, 2, 0);
> +                if ((ret = init_vlc(&s->dc_vlc[i], 11, 32,
> +                                    &dc_bias[i][0][1], 4, 2,
> +                                    &dc_bias[i][0][0], 4, 2, 0)) < 0)
> +                    return ret;
>  
>                  /* group 1 AC histograms */
> -                init_vlc(&s->ac_vlc_1[i], 11, 32,
> -                         &ac_bias_0[i][0][1], 4, 2,
> -                         &ac_bias_0[i][0][0], 4, 2, 0);
> +                if ((ret = init_vlc(&s->ac_vlc_1[i], 11, 32,
> +                                    &ac_bias_0[i][0][1], 4, 2,
> +                                    &ac_bias_0[i][0][0], 4, 2, 0)) < 0)
> +                    return ret;
>  
>                  /* group 2 AC histograms */
> -                init_vlc(&s->ac_vlc_2[i], 11, 32,
> -                         &ac_bias_1[i][0][1], 4, 2,
> -                         &ac_bias_1[i][0][0], 4, 2, 0);
> +                if ((ret = init_vlc(&s->ac_vlc_2[i], 11, 32,
> +                                    &ac_bias_1[i][0][1], 4, 2,
> +                                    &ac_bias_1[i][0][0], 4, 2, 0)) < 0)
> +                    return ret;
>  
>                  /* group 3 AC histograms */
> -                init_vlc(&s->ac_vlc_3[i], 11, 32,
> -                         &ac_bias_2[i][0][1], 4, 2,
> -                         &ac_bias_2[i][0][0], 4, 2, 0);
> +                if ((ret = init_vlc(&s->ac_vlc_3[i], 11, 32,
> +                                    &ac_bias_2[i][0][1], 4, 2,
> +                                    &ac_bias_2[i][0][0], 4, 2, 0)) < 0)
> +                    return ret;
>  
>                  /* group 4 AC histograms */
> -                init_vlc(&s->ac_vlc_4[i], 11, 32,
> -                         &ac_bias_3[i][0][1], 4, 2,
> -                         &ac_bias_3[i][0][0], 4, 2, 0);
> +                if ((ret = init_vlc(&s->ac_vlc_4[i], 11, 32,
> +                                    &ac_bias_3[i][0][1], 4, 2,
> +                                    &ac_bias_3[i][0][0], 4, 2, 0)) < 0)
> +                    return ret;
>              }
>  #if CONFIG_VP4_DECODER
>          } else { /* version >= 2 */
>              for (i = 0; i < 16; i++) {
>                  /* DC histograms */
> -                init_vlc(&s->dc_vlc[i], 11, 32,
> -                         &vp4_dc_bias[i][0][1], 4, 2,
> -                         &vp4_dc_bias[i][0][0], 4, 2, 0);
> +                if ((ret = init_vlc(&s->dc_vlc[i], 11, 32,
> +                                    &vp4_dc_bias[i][0][1], 4, 2,
> +                                    &vp4_dc_bias[i][0][0], 4, 2, 0)) < 0)
> +                    return ret;
>  
>                  /* group 1 AC histograms */
> -                init_vlc(&s->ac_vlc_1[i], 11, 32,
> -                         &vp4_ac_bias_0[i][0][1], 4, 2,
> -                         &vp4_ac_bias_0[i][0][0], 4, 2, 0);
> +                if ((ret = init_vlc(&s->ac_vlc_1[i], 11, 32,
> +                                    &vp4_ac_bias_0[i][0][1], 4, 2,
> +                                    &vp4_ac_bias_0[i][0][0], 4, 2, 0)) < 0)
> +                    return ret;
>  
>                  /* group 2 AC histograms */
> -                init_vlc(&s->ac_vlc_2[i], 11, 32,
> -                         &vp4_ac_bias_1[i][0][1], 4, 2,
> -                         &vp4_ac_bias_1[i][0][0], 4, 2, 0);
> +                if ((ret = init_vlc(&s->ac_vlc_2[i], 11, 32,
> +                                    &vp4_ac_bias_1[i][0][1], 4, 2,
> +                                    &vp4_ac_bias_1[i][0][0], 4, 2, 0)) < 0)
> +                    return ret;
>  
>                  /* group 3 AC histograms */
> -                init_vlc(&s->ac_vlc_3[i], 11, 32,
> -                         &vp4_ac_bias_2[i][0][1], 4, 2,
> -                         &vp4_ac_bias_2[i][0][0], 4, 2, 0);
> +                if ((ret = init_vlc(&s->ac_vlc_3[i], 11, 32,
> +                                    &vp4_ac_bias_2[i][0][1], 4, 2,
> +                                    &vp4_ac_bias_2[i][0][0], 4, 2, 0)) < 0)
> +                    return ret;
>  
>                  /* group 4 AC histograms */
> -                init_vlc(&s->ac_vlc_4[i], 11, 32,
> -                         &vp4_ac_bias_3[i][0][1], 4, 2,
> -                         &vp4_ac_bias_3[i][0][0], 4, 2, 0);
> +                if ((ret = init_vlc(&s->ac_vlc_4[i], 11, 32,
> +                                    &vp4_ac_bias_3[i][0][1], 4, 2,
> +                                    &vp4_ac_bias_3[i][0][0], 4, 2, 0)) < 0)
> +                    return ret;
>              }
>  #endif
>          }
> @@ -2502,34 +2512,40 @@ static av_cold int vp3_decode_init(AVCodecContext *avctx)
>          }
>      }
>  
> -    init_vlc(&s->superblock_run_length_vlc, 6, 34,
> -             &superblock_run_length_vlc_table[0][1], 4, 2,
> -             &superblock_run_length_vlc_table[0][0], 4, 2, 0);
> +    if ((ret = init_vlc(&s->superblock_run_length_vlc, 6, 34,
> +                        &superblock_run_length_vlc_table[0][1], 4, 2,
> +                        &superblock_run_length_vlc_table[0][0], 4, 2, 0)) < 0)
> +        return ret;
>  
> -    init_vlc(&s->fragment_run_length_vlc, 5, 30,
> -             &fragment_run_length_vlc_table[0][1], 4, 2,
> -             &fragment_run_length_vlc_table[0][0], 4, 2, 0);
> +    if ((ret = init_vlc(&s->fragment_run_length_vlc, 5, 30,
> +                        &fragment_run_length_vlc_table[0][1], 4, 2,
> +                        &fragment_run_length_vlc_table[0][0], 4, 2, 0)) < 0)
> +        return ret;
>  
> -    init_vlc(&s->mode_code_vlc, 3, 8,
> -             &mode_code_vlc_table[0][1], 2, 1,
> -             &mode_code_vlc_table[0][0], 2, 1, 0);
> +    if ((ret = init_vlc(&s->mode_code_vlc, 3, 8,
> +                        &mode_code_vlc_table[0][1], 2, 1,
> +                        &mode_code_vlc_table[0][0], 2, 1, 0)) < 0)
> +        return ret;
>  
> -    init_vlc(&s->motion_vector_vlc, 6, 63,
> -             &motion_vector_vlc_table[0][1], 2, 1,
> -             &motion_vector_vlc_table[0][0], 2, 1, 0);
> +    if ((ret = init_vlc(&s->motion_vector_vlc, 6, 63,
> +                        &motion_vector_vlc_table[0][1], 2, 1,
> +                        &motion_vector_vlc_table[0][0], 2, 1, 0)) < 0)
> +        return ret;
>  
>  #if CONFIG_VP4_DECODER
>      for (j = 0; j < 2; j++)
>          for (i = 0; i < 7; i++)
> -            init_vlc(&s->vp4_mv_vlc[j][i], 6, 63,
> -                 &vp4_mv_vlc[j][i][0][1], 4, 2,
> -                 &vp4_mv_vlc[j][i][0][0], 4, 2, 0);
> +            if ((ret = init_vlc(&s->vp4_mv_vlc[j][i], 6, 63,
> +                                &vp4_mv_vlc[j][i][0][1], 4, 2,
> +                                &vp4_mv_vlc[j][i][0][0], 4, 2, 0)) < 0)
> +                return ret;
>  
>      /* version >= 2 */
>      for (i = 0; i < 2; i++)
> -        init_vlc(&s->block_pattern_vlc[i], 3, 14,
> -             &vp4_block_pattern_vlc[i][0][1], 2, 1,
> -             &vp4_block_pattern_vlc[i][0][0], 2, 1, 0);
> +        if ((ret = init_vlc(&s->block_pattern_vlc[i], 3, 14,
> +                            &vp4_block_pattern_vlc[i][0][1], 2, 1,
> +                            &vp4_block_pattern_vlc[i][0][0], 2, 1, 0)) < 0)
> +            return ret;
>  #endif
>  
>      return allocate_tables(avctx);
> -- 
> 2.25.1

looks good

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
Andreas Rheinhardt Oct. 21, 2020, 4:15 a.m. UTC | #2
Peter Ross:
> On Tue, Oct 20, 2020 at 09:53:53AM +0200, Andreas Rheinhardt wrote:
>> It would lead to crashs lateron if they failed.
> 
> did you find these automatically?
> 
> have previously used this tool tool to find such errors: https://github.com/ralight/mallocfail
> 

No, I just look at the code and afterwards confirm (or refute) my
suspicions by making a specific command fail (by adding av_max_alloc(1);
before and av_max_alloc(INT_MAX); after the call).
Automating this would be nice, of course.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c
index 0d8d1ff80f..7ee65c8062 100644
--- a/libavcodec/vp3.c
+++ b/libavcodec/vp3.c
@@ -2414,57 +2414,67 @@  static av_cold int vp3_decode_init(AVCodecContext *avctx)
         if (s->version < 2) {
             for (i = 0; i < 16; i++) {
                 /* DC histograms */
-                init_vlc(&s->dc_vlc[i], 11, 32,
-                         &dc_bias[i][0][1], 4, 2,
-                         &dc_bias[i][0][0], 4, 2, 0);
+                if ((ret = init_vlc(&s->dc_vlc[i], 11, 32,
+                                    &dc_bias[i][0][1], 4, 2,
+                                    &dc_bias[i][0][0], 4, 2, 0)) < 0)
+                    return ret;
 
                 /* group 1 AC histograms */
-                init_vlc(&s->ac_vlc_1[i], 11, 32,
-                         &ac_bias_0[i][0][1], 4, 2,
-                         &ac_bias_0[i][0][0], 4, 2, 0);
+                if ((ret = init_vlc(&s->ac_vlc_1[i], 11, 32,
+                                    &ac_bias_0[i][0][1], 4, 2,
+                                    &ac_bias_0[i][0][0], 4, 2, 0)) < 0)
+                    return ret;
 
                 /* group 2 AC histograms */
-                init_vlc(&s->ac_vlc_2[i], 11, 32,
-                         &ac_bias_1[i][0][1], 4, 2,
-                         &ac_bias_1[i][0][0], 4, 2, 0);
+                if ((ret = init_vlc(&s->ac_vlc_2[i], 11, 32,
+                                    &ac_bias_1[i][0][1], 4, 2,
+                                    &ac_bias_1[i][0][0], 4, 2, 0)) < 0)
+                    return ret;
 
                 /* group 3 AC histograms */
-                init_vlc(&s->ac_vlc_3[i], 11, 32,
-                         &ac_bias_2[i][0][1], 4, 2,
-                         &ac_bias_2[i][0][0], 4, 2, 0);
+                if ((ret = init_vlc(&s->ac_vlc_3[i], 11, 32,
+                                    &ac_bias_2[i][0][1], 4, 2,
+                                    &ac_bias_2[i][0][0], 4, 2, 0)) < 0)
+                    return ret;
 
                 /* group 4 AC histograms */
-                init_vlc(&s->ac_vlc_4[i], 11, 32,
-                         &ac_bias_3[i][0][1], 4, 2,
-                         &ac_bias_3[i][0][0], 4, 2, 0);
+                if ((ret = init_vlc(&s->ac_vlc_4[i], 11, 32,
+                                    &ac_bias_3[i][0][1], 4, 2,
+                                    &ac_bias_3[i][0][0], 4, 2, 0)) < 0)
+                    return ret;
             }
 #if CONFIG_VP4_DECODER
         } else { /* version >= 2 */
             for (i = 0; i < 16; i++) {
                 /* DC histograms */
-                init_vlc(&s->dc_vlc[i], 11, 32,
-                         &vp4_dc_bias[i][0][1], 4, 2,
-                         &vp4_dc_bias[i][0][0], 4, 2, 0);
+                if ((ret = init_vlc(&s->dc_vlc[i], 11, 32,
+                                    &vp4_dc_bias[i][0][1], 4, 2,
+                                    &vp4_dc_bias[i][0][0], 4, 2, 0)) < 0)
+                    return ret;
 
                 /* group 1 AC histograms */
-                init_vlc(&s->ac_vlc_1[i], 11, 32,
-                         &vp4_ac_bias_0[i][0][1], 4, 2,
-                         &vp4_ac_bias_0[i][0][0], 4, 2, 0);
+                if ((ret = init_vlc(&s->ac_vlc_1[i], 11, 32,
+                                    &vp4_ac_bias_0[i][0][1], 4, 2,
+                                    &vp4_ac_bias_0[i][0][0], 4, 2, 0)) < 0)
+                    return ret;
 
                 /* group 2 AC histograms */
-                init_vlc(&s->ac_vlc_2[i], 11, 32,
-                         &vp4_ac_bias_1[i][0][1], 4, 2,
-                         &vp4_ac_bias_1[i][0][0], 4, 2, 0);
+                if ((ret = init_vlc(&s->ac_vlc_2[i], 11, 32,
+                                    &vp4_ac_bias_1[i][0][1], 4, 2,
+                                    &vp4_ac_bias_1[i][0][0], 4, 2, 0)) < 0)
+                    return ret;
 
                 /* group 3 AC histograms */
-                init_vlc(&s->ac_vlc_3[i], 11, 32,
-                         &vp4_ac_bias_2[i][0][1], 4, 2,
-                         &vp4_ac_bias_2[i][0][0], 4, 2, 0);
+                if ((ret = init_vlc(&s->ac_vlc_3[i], 11, 32,
+                                    &vp4_ac_bias_2[i][0][1], 4, 2,
+                                    &vp4_ac_bias_2[i][0][0], 4, 2, 0)) < 0)
+                    return ret;
 
                 /* group 4 AC histograms */
-                init_vlc(&s->ac_vlc_4[i], 11, 32,
-                         &vp4_ac_bias_3[i][0][1], 4, 2,
-                         &vp4_ac_bias_3[i][0][0], 4, 2, 0);
+                if ((ret = init_vlc(&s->ac_vlc_4[i], 11, 32,
+                                    &vp4_ac_bias_3[i][0][1], 4, 2,
+                                    &vp4_ac_bias_3[i][0][0], 4, 2, 0)) < 0)
+                    return ret;
             }
 #endif
         }
@@ -2502,34 +2512,40 @@  static av_cold int vp3_decode_init(AVCodecContext *avctx)
         }
     }
 
-    init_vlc(&s->superblock_run_length_vlc, 6, 34,
-             &superblock_run_length_vlc_table[0][1], 4, 2,
-             &superblock_run_length_vlc_table[0][0], 4, 2, 0);
+    if ((ret = init_vlc(&s->superblock_run_length_vlc, 6, 34,
+                        &superblock_run_length_vlc_table[0][1], 4, 2,
+                        &superblock_run_length_vlc_table[0][0], 4, 2, 0)) < 0)
+        return ret;
 
-    init_vlc(&s->fragment_run_length_vlc, 5, 30,
-             &fragment_run_length_vlc_table[0][1], 4, 2,
-             &fragment_run_length_vlc_table[0][0], 4, 2, 0);
+    if ((ret = init_vlc(&s->fragment_run_length_vlc, 5, 30,
+                        &fragment_run_length_vlc_table[0][1], 4, 2,
+                        &fragment_run_length_vlc_table[0][0], 4, 2, 0)) < 0)
+        return ret;
 
-    init_vlc(&s->mode_code_vlc, 3, 8,
-             &mode_code_vlc_table[0][1], 2, 1,
-             &mode_code_vlc_table[0][0], 2, 1, 0);
+    if ((ret = init_vlc(&s->mode_code_vlc, 3, 8,
+                        &mode_code_vlc_table[0][1], 2, 1,
+                        &mode_code_vlc_table[0][0], 2, 1, 0)) < 0)
+        return ret;
 
-    init_vlc(&s->motion_vector_vlc, 6, 63,
-             &motion_vector_vlc_table[0][1], 2, 1,
-             &motion_vector_vlc_table[0][0], 2, 1, 0);
+    if ((ret = init_vlc(&s->motion_vector_vlc, 6, 63,
+                        &motion_vector_vlc_table[0][1], 2, 1,
+                        &motion_vector_vlc_table[0][0], 2, 1, 0)) < 0)
+        return ret;
 
 #if CONFIG_VP4_DECODER
     for (j = 0; j < 2; j++)
         for (i = 0; i < 7; i++)
-            init_vlc(&s->vp4_mv_vlc[j][i], 6, 63,
-                 &vp4_mv_vlc[j][i][0][1], 4, 2,
-                 &vp4_mv_vlc[j][i][0][0], 4, 2, 0);
+            if ((ret = init_vlc(&s->vp4_mv_vlc[j][i], 6, 63,
+                                &vp4_mv_vlc[j][i][0][1], 4, 2,
+                                &vp4_mv_vlc[j][i][0][0], 4, 2, 0)) < 0)
+                return ret;
 
     /* version >= 2 */
     for (i = 0; i < 2; i++)
-        init_vlc(&s->block_pattern_vlc[i], 3, 14,
-             &vp4_block_pattern_vlc[i][0][1], 2, 1,
-             &vp4_block_pattern_vlc[i][0][0], 2, 1, 0);
+        if ((ret = init_vlc(&s->block_pattern_vlc[i], 3, 14,
+                            &vp4_block_pattern_vlc[i][0][1], 2, 1,
+                            &vp4_block_pattern_vlc[i][0][0], 2, 1, 0)) < 0)
+            return ret;
 #endif
 
     return allocate_tables(avctx);