On Tue, Aug 06, 2024 at 07:54:58PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Tue, Aug 06, 2024 at 07:05:38PM +0200, Michael Niedermayer wrote:
> >> Hi
> >>
> >> Did CBS win the obfuscated C contest yet?
> >>
> >> I was just looking at a msan issue and then looked at this:
> >>
> >> CHECK(FUNC_SEI(message_list)(ctx, rw, &current->message_list, 1));
> >>
> >>
> >> #define CHECK(call) do { \
> >>         err = (call); \
> >>         if (err < 0) \
> >>             return err; \
> >>     } while (0)
> >>
> >> #define FUNC_NAME2(rw, codec, name) cbs_ ## codec ## _ ## rw ## _ ## name
> >> #define FUNC_NAME1(rw, codec, name) FUNC_NAME2(rw, codec, name)
> >> #define FUNC_H264(name) FUNC_NAME1(READWRITE, h264, name)
> >> #define FUNC_H265(name) FUNC_NAME1(READWRITE, h265, name)
> >> #define FUNC_H266(name) FUNC_NAME1(READWRITE, h266, name)
> >> #define FUNC_SEI(name)  FUNC_NAME1(READWRITE, sei,  name)
> >>
> >> #define SEI_FUNC(name, args) \
> >> static int FUNC(name) args;  \
> >> static int FUNC(name ## _internal)(CodedBitstreamContext *ctx, \
> >>                                    RWContext *rw, void *cur,   \
> >>                                    SEIMessageState *state)     \
> >> { \
> >>     return FUNC(name)(ctx, rw, cur, state); \
> >> } \
> >> static int FUNC(name) args
> >>
> >>
> >> anyway, can we remove all preprocessor use from cbs ?
> 
> I don't think that this is really obfuscated.

Then you qualify as maintainer for this code.
Thanks!


> 
> > 
> > the issue iam looking at is due to
> > 
> > SEI_FUNC(sei_pic_timing, (CodedBitstreamContext *ctx, RWContext *rw, H264RawSEIPicTiming *current, SEIMessageState *sei))
> > 
> > having different active SPS on writing than reading, so the write code
> > has nal_hrd_parameters_present_flag set while the read had that 0
> > so uninitialized data is written
> > 
> > I cannot find any match for "cbs" in MAINTAINERS, also there are no copyright
> > with names in the cbs code.
> 
> 1. I just sent a patch that "fixes" this.

Thats what i came up with too (and it works). Given we both hit that,
please apply


> 2. But actually, there is a deeper bug here: We would need to defer
> parsing certain SEI message units to a second pass when the currently
> active SPS is known. This can happen with spec-compliant input (and even
> more so with spec-incompliant input, which is presumably what the fuzzer
> produced).

Thats a deeper analysis than what i did. I stoped at the "just clear it, as in 1."
and then was looking for someone who would implement the correct fix

Ill send you the input sample privately

Thanks alot!

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle