Re: [FFmpeg-devel] [PATCH 2/3] avcodec/cbs_h266_syntax_template: sanity check num_multi_layer_olss

From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 2/3] avcodec/cbs_h266_syntax_template: sanity check num_multi_layer_olss
Date: Sun, 28 Jan 2024 01:05:20 +0100
Message-ID: <20240128000520.GM6420@pb2> (raw)
In-Reply-To: <9742e81b-0a57-4bd4-bcb9-b68ce006cc67@gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 2701 bytes --]

On Sat, Jan 27, 2024 at 09:02:30PM -0300, James Almer wrote:
> On 1/27/2024 8:56 PM, Michael Niedermayer wrote:
> > On Sat, Jan 27, 2024 at 09:25:16AM -0300, James Almer wrote:
> > > On 1/26/2024 6:46 PM, Michael Niedermayer wrote:
> > > > It is not possible to encode a index into an empty list. Thus
> > > > this must be invalid at this point or before.
> > > > Its likely a broader earlier check can be used here, someone knowing
> > > > VVC should look at that. Its not immedeatly obvious from the spec
> > > > by looking for numlayerolss
> > > 
> > > Can you check if the following fixes it?
> > > 
> > > > diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c
> > > > index 549d021211..40572dadb5 100644
> > > > --- a/libavcodec/cbs_h266_syntax_template.c
> > > > +++ b/libavcodec/cbs_h266_syntax_template.c
> > > > @@ -793,6 +793,7 @@ static int FUNC(vps) (CodedBitstreamContext *ctx, RWContext *rw,
> > > >       {
> > > >           //calc NumMultiLayerOlss
> > > >           int m;
> > > > +        int num_layers_in_ols = 0;
> > > >           uint8_t dependency_flag[VVC_MAX_LAYERS][VVC_MAX_LAYERS];
> > > >           uint16_t num_output_layers_in_ols[VVC_MAX_TOTAL_NUM_OLSS];
> > > >           uint8_t num_sub_layers_in_layer_in_ols[VVC_MAX_TOTAL_NUM_OLSS][VVC_MAX_TOTAL_NUM_OLSS];
> > > > @@ -895,7 +896,6 @@ static int FUNC(vps) (CodedBitstreamContext *ctx, RWContext *rw,
> > > >                   return AVERROR_INVALIDDATA;
> > > >           }
> > > >           for (i = 1; i < total_num_olss; i++) {
> > > > -            int num_layers_in_ols = 0;
> > > >               if (current->vps_each_layer_is_an_ols_flag) {
> > > >                   num_layers_in_ols = 1;
> > > >               } else if (current->vps_ols_mode_idc == 0 ||
> > > 
> > > num_layers_in_ols is not meant to be reset on every loop.
> > 
> > replacing my patch by yours does not change
> > num_multi_layer_olss from being 0
> > and if its 0 then "num_multi_layer_olss - 1" causes problems as a limit
> > 
> > more precissely this:
> > i can also send you the file if you want?
> 
> No, this should be looked at by someone more familiar with VVC.

ive already sent the fuzzer samples to nuomi and frank plowman

> And my patch should be applied either way. The current code is wrong.

I did not suggest not to do that :)
just that it alone was not enough to fix this

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Take away the freedom of one citizen and you will be jailed, take away
the freedom of all citizens and you will be congratulated by your peers
in Parliament.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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