| 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 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
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)
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 --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);
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(-)