Message ID | 20210116052434.877-1-lq@chinaffmpeg.org |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel] avcodec/bsf: set pctx to NULL when av_bsf_alloc failed | expand |
Context | Check | Description |
---|---|---|
andriy/x86_make | success | Make finished |
andriy/x86_make_fate | success | Make fate finished |
andriy/PPC64_make | success | Make finished |
andriy/PPC64_make_fate | success | Make fate finished |
On 1/16/2021 2:24 AM, Steven Liu wrote: > av_bsf_free will free invalid pointer when av_bsf_alloc failed. > because av_bsf_list_parse_str called av_bsf_get_null_filter, > av_bsf_get_null_filter called av_bsf_alloc, and av_bsf_alloc > should set a value to the *pctx before return success or failed, > because it dose not initial a null pointer ever, so it will free > invalid pointer in av_bsf_free which is called by ff_decode_bsfs_init. The pointer passed to av_bsf_list_parse_str() in ff_decode_bsfs_init() is already NULL, because avctx->internal was allocated with av_mallocz(). In what scenario is av_bsf_free() getting an invalid pointer? > > Found-by: Zu-Ming Jiang <jjzuming@outlook.com> > Signed-off-by: Steven Liu <lq@chinaffmpeg.org> > --- > libavcodec/bsf.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libavcodec/bsf.c b/libavcodec/bsf.c > index d71bc32584..5bb3349138 100644 > --- a/libavcodec/bsf.c > +++ b/libavcodec/bsf.c > @@ -141,6 +141,7 @@ int av_bsf_alloc(const AVBitStreamFilter *filter, AVBSFContext **pctx) > return 0; > fail: > av_bsf_free(&ctx); > + *pctx = NULL; > return ret; > } > >
> 在 2021年1月16日,21:19,James Almer <jamrial@gmail.com> 写道: > > On 1/16/2021 2:24 AM, Steven Liu wrote: >> av_bsf_free will free invalid pointer when av_bsf_alloc failed. >> because av_bsf_list_parse_str called av_bsf_get_null_filter, >> av_bsf_get_null_filter called av_bsf_alloc, and av_bsf_alloc >> should set a value to the *pctx before return success or failed, >> because it dose not initial a null pointer ever, so it will free >> invalid pointer in av_bsf_free which is called by ff_decode_bsfs_init. > > The pointer passed to av_bsf_list_parse_str() in ff_decode_bsfs_init() is already NULL, because avctx->internal was allocated with av_mallocz(). It’s good point, yes, you are right. > > In what scenario is av_bsf_free() getting an invalid pointer? only call ff_decode_bsfs_init and not use avcodec_open2, call it by myself write internal code, maybe nobody will use it like my way :( > >> Found-by: Zu-Ming Jiang <jjzuming@outlook.com> >> Signed-off-by: Steven Liu <lq@chinaffmpeg.org> >> --- >> libavcodec/bsf.c | 1 + >> 1 file changed, 1 insertion(+) >> diff --git a/libavcodec/bsf.c b/libavcodec/bsf.c >> index d71bc32584..5bb3349138 100644 >> --- a/libavcodec/bsf.c >> +++ b/libavcodec/bsf.c >> @@ -141,6 +141,7 @@ int av_bsf_alloc(const AVBitStreamFilter *filter, AVBSFContext **pctx) >> return 0; >> fail: >> av_bsf_free(&ctx); >> + *pctx = NULL; >> return ret; >> } >> > > _______________________________________________ > 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". Thanks Steven
diff --git a/libavcodec/bsf.c b/libavcodec/bsf.c index d71bc32584..5bb3349138 100644 --- a/libavcodec/bsf.c +++ b/libavcodec/bsf.c @@ -141,6 +141,7 @@ int av_bsf_alloc(const AVBitStreamFilter *filter, AVBSFContext **pctx) return 0; fail: av_bsf_free(&ctx); + *pctx = NULL; return ret; }
av_bsf_free will free invalid pointer when av_bsf_alloc failed. because av_bsf_list_parse_str called av_bsf_get_null_filter, av_bsf_get_null_filter called av_bsf_alloc, and av_bsf_alloc should set a value to the *pctx before return success or failed, because it dose not initial a null pointer ever, so it will free invalid pointer in av_bsf_free which is called by ff_decode_bsfs_init. Found-by: Zu-Ming Jiang <jjzuming@outlook.com> Signed-off-by: Steven Liu <lq@chinaffmpeg.org> --- libavcodec/bsf.c | 1 + 1 file changed, 1 insertion(+)