[FFmpeg-devel] MpegEncContext->class is never initialized

[FFmpeg-devel] MpegEncContext->class is never initialized

[John Dorian]

I discovered a crash here if log handler function tries to get the class
name from "s"

mpeg12dec.c:
if (get_bits_left(&s->gb) < 0) {
         av_log(s, AV_LOG_ERROR, "overread %d\n", -get_bits_left(&s->gb));
         return AVERROR_INVALIDDATA;
     }

And it seems to be because MpegEncContext->class is never initialized (or
rather it is initialized to 0) due to av_malloc_z() I assume.

So the question is, is it valid for any av object to have its av_class set
to 0.
_______________________________________________
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".

Re: [FFmpeg-devel] MpegEncContext->class is never initialized

[Andreas Rheinhardt]

John Dorian:
> I discovered a crash here if log handler function tries to get the class
> name from "s"
> 
> mpeg12dec.c:
> if (get_bits_left(&s->gb) < 0) {
>          av_log(s, AV_LOG_ERROR, "overread %d\n", -get_bits_left(&s->gb));
>          return AVERROR_INVALIDDATA;
>      }
> 
> And it seems to be because MpegEncContext->class is never initialized (or
> rather it is initialized to 0) due to av_malloc_z() I assume.
> 
> So the question is, is it valid for any av object to have its av_class set
> to 0.

Will send a fix shortly.

- Andreas

_______________________________________________
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".

Re: [FFmpeg-devel] MpegEncContext->class is never initialized

[Andreas Rheinhardt]

John Dorian:
> I discovered a crash here if log handler function tries to get the class
> name from "s"
> 
> mpeg12dec.c:
> if (get_bits_left(&s->gb) < 0) {
>          av_log(s, AV_LOG_ERROR, "overread %d\n", -get_bits_left(&s->gb));
>          return AVERROR_INVALIDDATA;
>      }
> 
> And it seems to be because MpegEncContext->class is never initialized (or
> rather it is initialized to 0) due to av_malloc_z() I assume.
> 
> So the question is, is it valid for any av object to have its av_class set
> to 0.

Here is the patch:
https://ffmpeg.org/pipermail/ffmpeg-devel/2025-March/340615.html
My reading of log.h is that it is illegal for the pointer to an AVClass
to be NULL. The default log callback handles this gracefully, so I
presume you are using a custom log callback.

- Andreas

_______________________________________________
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".

Re: [FFmpeg-devel] MpegEncContext->class is never initialized

[John Dorian]

Yes, I have a custom log callback. I have multiple files open and I'd like
to associate logged errors with the correct file, which I am doing through
the objects that have an "opaque" member (not all, but most do). I don't
think this is any abuse of the API as it does state that the first member
of a struct passed through av_log should be AVClass*.

The solution for now is to check for a null pointer e.g. in the log
callback, "if ((AVClass**)ptr[0])==NULL)" The helper log functions such as
av_default_item_name() do not have this check, perhaps that is the real
problem here.

I'm not so sure it is required to be non-null given that the features of
AVClass may not be useful to every object, e.g. options and such may not be
appropriate for private objects.


On Tue, Mar 4, 2025 at 4:36 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> John Dorian:
> > I discovered a crash here if log handler function tries to get the class
> > name from "s"
> >
> > mpeg12dec.c:
> > if (get_bits_left(&s->gb) < 0) {
> >          av_log(s, AV_LOG_ERROR, "overread %d\n",
> -get_bits_left(&s->gb));
> >          return AVERROR_INVALIDDATA;
> >      }
> >
> > And it seems to be because MpegEncContext->class is never initialized (or
> > rather it is initialized to 0) due to av_malloc_z() I assume.
> >
> > So the question is, is it valid for any av object to have its av_class
> set
> > to 0.
>
> Here is the patch:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2025-March/340615.html
> My reading of log.h is that it is illegal for the pointer to an AVClass
> to be NULL. The default log callback handles this gracefully, so I
> presume you are using a custom log callback.
>
> - Andreas
>
> _______________________________________________
> 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".
>
_______________________________________________
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".

Re: [FFmpeg-devel] MpegEncContext->class is never initialized

[Andreas Rheinhardt]

John Dorian:
> Yes, I have a custom log callback. I have multiple files open and I'd like
> to associate logged errors with the correct file, which I am doing through
> the objects that have an "opaque" member (not all, but most do). I don't
> think this is any abuse of the API as it does state that the first member
> of a struct passed through av_log should be AVClass*.
> 

I meant: We (as in the libavcodec library/the FFmpeg project) don't
abide by our own APIs when we pass objects with a NULL AVClass* to av_log().

> The solution for now is to check for a null pointer e.g. in the log
> callback, "if ((AVClass**)ptr[0])==NULL)" The helper log functions such as
> av_default_item_name() do not have this check, perhaps that is the real
> problem here.
> 

Here is how the default log callback handles this:
    AVClass* avc = avcl ? *(AVClass **) avcl : NULL;
Further down the case of no object and an object with AVClass* set to
NULL are therefore handled the same.

> I'm not so sure it is required to be non-null given that the features of
> AVClass may not be useful to every object, e.g. options and such may not be
> appropriate for private objects.
> 

If no AVClass* is set, then this object should not be used with the
av_log API.

> 
> On Tue, Mar 4, 2025 at 4:36 PM Andreas Rheinhardt <
> andreas.rheinhardt@outlook.com> wrote:
> 
>> John Dorian:
>>> I discovered a crash here if log handler function tries to get the class
>>> name from "s"
>>>
>>> mpeg12dec.c:
>>> if (get_bits_left(&s->gb) < 0) {
>>>          av_log(s, AV_LOG_ERROR, "overread %d\n",
>> -get_bits_left(&s->gb));
>>>          return AVERROR_INVALIDDATA;
>>>      }
>>>
>>> And it seems to be because MpegEncContext->class is never initialized (or
>>> rather it is initialized to 0) due to av_malloc_z() I assume.
>>>
>>> So the question is, is it valid for any av object to have its av_class
>> set
>>> to 0.
>>
>> Here is the patch:
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2025-March/340615.html
>> My reading of log.h is that it is illegal for the pointer to an AVClass
>> to be NULL. The default log callback handles this gracefully, so I
>> presume you are using a custom log callback.
>>
>> - Andreas
>>
_______________________________________________
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".