| Message ID | 20220215095908.1672123-1-jiasheng@iscas.ac.cn |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/mlz: Add the check after calling av_mallocz | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
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
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 */
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(-)