diff mbox series

[FFmpeg-devel,2/5] avcodec/allcodecs: Dont play with NULLs

Message ID 20220321201946.16919-2-michael@niedermayer.cc
State Accepted
Commit dd1ce72e68f77c88fe8d258d83f7315c34b1eb11
Headers show
Series [FFmpeg-devel,1/5] avcodec/vp9_superframe_bsf: Check in size | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished

Commit Message

Michael Niedermayer March 21, 2022, 8:19 p.m. UTC
Fixes: member access within null pointer of type 'const FFCodec' (aka 'const struct FFCodec')
Fixes: 45726/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6554445419249664

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/allcodecs.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

James Almer March 21, 2022, 8:33 p.m. UTC | #1
On 3/21/2022 5:19 PM, Michael Niedermayer wrote:
> Fixes: member access within null pointer of type 'const FFCodec' (aka 'const struct FFCodec')
> Fixes: 45726/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6554445419249664
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavcodec/allcodecs.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index b1aa7e266b..22d56760ec 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -882,10 +882,11 @@ const AVCodec *av_codec_iterate(void **opaque)
>   
>       ff_thread_once(&av_codec_static_init, av_codec_init_static);
>   
> -    if (c)
> +    if (c) {
>           *opaque = (void*)(i + 1);
> -
> -    return &c->p;
> +        return &c->p;
> +    }
> +    return NULL;

Can't you just do

return (const AVCodec *)c;

Or is that aliasing a problem?

>   }
>   
>   static enum AVCodecID remap_deprecated_codec_id(enum AVCodecID id)
Andreas Rheinhardt March 21, 2022, 8:48 p.m. UTC | #2
Michael Niedermayer:
> Fixes: member access within null pointer of type 'const FFCodec' (aka 'const struct FFCodec')
> Fixes: 45726/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6554445419249664
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/allcodecs.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index b1aa7e266b..22d56760ec 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -882,10 +882,11 @@ const AVCodec *av_codec_iterate(void **opaque)
>  
>      ff_thread_once(&av_codec_static_init, av_codec_init_static);
>  
> -    if (c)
> +    if (c) {
>          *opaque = (void*)(i + 1);
> -
> -    return &c->p;
> +        return &c->p;
> +    }
> +    return NULL;
>  }
>  
>  static enum AVCodecID remap_deprecated_codec_id(enum AVCodecID id)

LGTM.

- Andreas
Andreas Rheinhardt March 21, 2022, 8:55 p.m. UTC | #3
James Almer:
> 
> 
> On 3/21/2022 5:19 PM, Michael Niedermayer wrote:
>> Fixes: member access within null pointer of type 'const FFCodec' (aka
>> 'const struct FFCodec')
>> Fixes:
>> 45726/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6554445419249664
>>
>>
>> Found-by: continuous fuzzing process
>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>> ---
>>   libavcodec/allcodecs.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> index b1aa7e266b..22d56760ec 100644
>> --- a/libavcodec/allcodecs.c
>> +++ b/libavcodec/allcodecs.c
>> @@ -882,10 +882,11 @@ const AVCodec *av_codec_iterate(void **opaque)
>>         ff_thread_once(&av_codec_static_init, av_codec_init_static);
>>   -    if (c)
>> +    if (c) {
>>           *opaque = (void*)(i + 1);
>> -
>> -    return &c->p;
>> +        return &c->p;
>> +    }
>> +    return NULL;
> 
> Can't you just do
> 
> return (const AVCodec *)c;
> 
> Or is that aliasing a problem?
> 

There is no aliasing problem: It is perfectly legal to cast a pointer to
a struct to a pointer to its first member. But it unnecessarily
circumvents the type system, so this patch here is better.

- Andreas
Michael Niedermayer March 21, 2022, 9:03 p.m. UTC | #4
On Mon, Mar 21, 2022 at 09:48:40PM +0100, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: member access within null pointer of type 'const FFCodec' (aka 'const struct FFCodec')
> > Fixes: 45726/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6554445419249664
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/allcodecs.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> > index b1aa7e266b..22d56760ec 100644
> > --- a/libavcodec/allcodecs.c
> > +++ b/libavcodec/allcodecs.c
> > @@ -882,10 +882,11 @@ const AVCodec *av_codec_iterate(void **opaque)
> >  
> >      ff_thread_once(&av_codec_static_init, av_codec_init_static);
> >  
> > -    if (c)
> > +    if (c) {
> >          *opaque = (void*)(i + 1);
> > -
> > -    return &c->p;
> > +        return &c->p;
> > +    }
> > +    return NULL;
> >  }
> >  
> >  static enum AVCodecID remap_deprecated_codec_id(enum AVCodecID id)
> 
> LGTM.

will apply

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index b1aa7e266b..22d56760ec 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -882,10 +882,11 @@  const AVCodec *av_codec_iterate(void **opaque)
 
     ff_thread_once(&av_codec_static_init, av_codec_init_static);
 
-    if (c)
+    if (c) {
         *opaque = (void*)(i + 1);
-
-    return &c->p;
+        return &c->p;
+    }
+    return NULL;
 }
 
 static enum AVCodecID remap_deprecated_codec_id(enum AVCodecID id)