| Message ID | 20240321011517.10363-4-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/4] avcodec/mscc: move frame allocates to later | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On 3/20/2024 10:15 PM, Michael Niedermayer wrote: > Fixes: null pointer dereference > Fixes: 67023/clusterfuzz-testcase-minimized-ffmpeg_dem_IAMF_fuzzer-6011025237278720 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/iamf.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/libavformat/iamf.c b/libavformat/iamf.c > index 5de70dc082..f2c22ce3aa 100644 > --- a/libavformat/iamf.c > +++ b/libavformat/iamf.c > @@ -89,9 +89,10 @@ void ff_iamf_free_mix_presentation(IAMFMixPresentation **pmix_presentation) > if (!mix_presentation) > return; > > - for (int i = 0; i < mix_presentation->count_label; i++) > - av_free(mix_presentation->language_label[i]); > - av_free(mix_presentation->language_label); > + if (mix_presentation->language_label) If count_label is not 0, then language_label should be allocated. > + for (int i = 0; i < mix_presentation->count_label; i++) > + av_free(mix_presentation->language_label[i]); > + av_freep(&mix_presentation->language_label); > av_iamf_mix_presentation_free(&mix_presentation->mix); > av_freep(pmix_presentation); > } Can you test the following? > diff --git a/libavformat/iamf_parse.c b/libavformat/iamf_parse.c > index cb49cf0a57..e29c2c6b6c 100644 > --- a/libavformat/iamf_parse.c > +++ b/libavformat/iamf_parse.c > @@ -822,6 +822,7 @@ static int mix_presentation_obu(void *s, IAMFContext *c, AVIOContext *pb, int le > mix_presentation->language_label = av_calloc(mix_presentation->count_label, > sizeof(*mix_presentation->language_label)); > if (!mix_presentation->language_label) { > + mix_presentation->count_label = 0; > ret = AVERROR(ENOMEM); > goto fail; > }
On Wed, Mar 20, 2024 at 11:17:09PM -0300, James Almer wrote: > On 3/20/2024 10:15 PM, Michael Niedermayer wrote: > > Fixes: null pointer dereference > > Fixes: 67023/clusterfuzz-testcase-minimized-ffmpeg_dem_IAMF_fuzzer-6011025237278720 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/iamf.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/libavformat/iamf.c b/libavformat/iamf.c > > index 5de70dc082..f2c22ce3aa 100644 > > --- a/libavformat/iamf.c > > +++ b/libavformat/iamf.c > > @@ -89,9 +89,10 @@ void ff_iamf_free_mix_presentation(IAMFMixPresentation **pmix_presentation) > > if (!mix_presentation) > > return; > > - for (int i = 0; i < mix_presentation->count_label; i++) > > - av_free(mix_presentation->language_label[i]); > > - av_free(mix_presentation->language_label); > > + if (mix_presentation->language_label) > > If count_label is not 0, then language_label should be allocated. > > > + for (int i = 0; i < mix_presentation->count_label; i++) > > + av_free(mix_presentation->language_label[i]); > > + av_freep(&mix_presentation->language_label); > > av_iamf_mix_presentation_free(&mix_presentation->mix); > > av_freep(pmix_presentation); > > } > > Can you test the following? > > > diff --git a/libavformat/iamf_parse.c b/libavformat/iamf_parse.c > > index cb49cf0a57..e29c2c6b6c 100644 > > --- a/libavformat/iamf_parse.c > > +++ b/libavformat/iamf_parse.c > > @@ -822,6 +822,7 @@ static int mix_presentation_obu(void *s, IAMFContext *c, AVIOContext *pb, int le > > mix_presentation->language_label = av_calloc(mix_presentation->count_label, > > sizeof(*mix_presentation->language_label)); > > if (!mix_presentation->language_label) { > > + mix_presentation->count_label = 0; > > ret = AVERROR(ENOMEM); > > goto fail; > > } that works too, i think pointers should be set to NULL on deallocation though thx [...]
On Thu, Mar 21, 2024 at 04:55:46AM +0100, Michael Niedermayer wrote: > On Wed, Mar 20, 2024 at 11:17:09PM -0300, James Almer wrote: > > On 3/20/2024 10:15 PM, Michael Niedermayer wrote: > > > Fixes: null pointer dereference > > > Fixes: 67023/clusterfuzz-testcase-minimized-ffmpeg_dem_IAMF_fuzzer-6011025237278720 > > > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > --- > > > libavformat/iamf.c | 7 ++++--- > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/libavformat/iamf.c b/libavformat/iamf.c > > > index 5de70dc082..f2c22ce3aa 100644 > > > --- a/libavformat/iamf.c > > > +++ b/libavformat/iamf.c > > > @@ -89,9 +89,10 @@ void ff_iamf_free_mix_presentation(IAMFMixPresentation **pmix_presentation) > > > if (!mix_presentation) > > > return; > > > - for (int i = 0; i < mix_presentation->count_label; i++) > > > - av_free(mix_presentation->language_label[i]); > > > - av_free(mix_presentation->language_label); > > > + if (mix_presentation->language_label) > > > > If count_label is not 0, then language_label should be allocated. > > > > > + for (int i = 0; i < mix_presentation->count_label; i++) > > > + av_free(mix_presentation->language_label[i]); > > > + av_freep(&mix_presentation->language_label); > > > av_iamf_mix_presentation_free(&mix_presentation->mix); > > > av_freep(pmix_presentation); > > > } > > > > Can you test the following? > > > > > diff --git a/libavformat/iamf_parse.c b/libavformat/iamf_parse.c > > > index cb49cf0a57..e29c2c6b6c 100644 > > > --- a/libavformat/iamf_parse.c > > > +++ b/libavformat/iamf_parse.c > > > @@ -822,6 +822,7 @@ static int mix_presentation_obu(void *s, IAMFContext *c, AVIOContext *pb, int le > > > mix_presentation->language_label = av_calloc(mix_presentation->count_label, > > > sizeof(*mix_presentation->language_label)); > > > if (!mix_presentation->language_label) { > > > + mix_presentation->count_label = 0; > > > ret = AVERROR(ENOMEM); > > > goto fail; > > > } > > that works too, i think pointers should be set to NULL on deallocation though i will apply this alternative txh [...]
diff --git a/libavformat/iamf.c b/libavformat/iamf.c index 5de70dc082..f2c22ce3aa 100644 --- a/libavformat/iamf.c +++ b/libavformat/iamf.c @@ -89,9 +89,10 @@ void ff_iamf_free_mix_presentation(IAMFMixPresentation **pmix_presentation) if (!mix_presentation) return; - for (int i = 0; i < mix_presentation->count_label; i++) - av_free(mix_presentation->language_label[i]); - av_free(mix_presentation->language_label); + if (mix_presentation->language_label) + for (int i = 0; i < mix_presentation->count_label; i++) + av_free(mix_presentation->language_label[i]); + av_freep(&mix_presentation->language_label); av_iamf_mix_presentation_free(&mix_presentation->mix); av_freep(pmix_presentation); }
Fixes: null pointer dereference Fixes: 67023/clusterfuzz-testcase-minimized-ffmpeg_dem_IAMF_fuzzer-6011025237278720 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/iamf.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)