[FFmpeg-devel] [PATCH 1/2] avcodec/vvc/refs: fix negative pps_scaling_win offsets

[FFmpeg-devel] [PATCH 1/2] avcodec/vvc/refs: fix negative pps_scaling_win offsets

[Michael Niedermayer]

The spec seems to allow these to  be negative

Fixes: left shift of negative value -15
Fixes: 392687035/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-6559804532785152

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

diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
index 8d4b7bb35b2..486515d06db 100644
--- a/libavcodec/vvc/refs.c
+++ b/libavcodec/vvc/refs.c
@@ -147,10 +147,10 @@ static VVCFrame *alloc_frame(VVCContext *s, VVCFrameContext *fc)
         for (int j = 0; j < frame->ctb_count; j++)
             frame->rpl_tab[j] = frame->rpl;
 
-        win->left_offset   = pps->r->pps_scaling_win_left_offset   << sps->hshift[CHROMA];
-        win->right_offset  = pps->r->pps_scaling_win_right_offset  << sps->hshift[CHROMA];
-        win->top_offset    = pps->r->pps_scaling_win_top_offset    << sps->vshift[CHROMA];
-        win->bottom_offset = pps->r->pps_scaling_win_bottom_offset << sps->vshift[CHROMA];
+        win->left_offset   = pps->r->pps_scaling_win_left_offset   * (1 << sps->hshift[CHROMA]);
+        win->right_offset  = pps->r->pps_scaling_win_right_offset  * (1 << sps->hshift[CHROMA]);
+        win->top_offset    = pps->r->pps_scaling_win_top_offset    * (1 << sps->vshift[CHROMA]);
+        win->bottom_offset = pps->r->pps_scaling_win_bottom_offset * (1 << sps->vshift[CHROMA]);
         frame->ref_width   = pps->r->pps_pic_width_in_luma_samples  - win->left_offset   - win->right_offset;
         frame->ref_height  = pps->r->pps_pic_height_in_luma_samples - win->bottom_offset - win->top_offset;
 
-- 
2.48.1

_______________________________________________
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] [PATCH 2/2] avcodec/vvc/refs: Check content_ref in set_pict_type()

[Michael Niedermayer]

Fixes: 390565846/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-4990028521996288
Fixes: Null pointer dereference

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/vvc/refs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
index 486515d06db..1cfca482047 100644
--- a/libavcodec/vvc/refs.c
+++ b/libavcodec/vvc/refs.c
@@ -186,7 +186,7 @@ static void set_pict_type(AVFrame *frame, const VVCContext *s, const VVCFrameCon
         const CodedBitstreamFragment *current = &s->current_frame;
         for (int i = 0; i < current->nb_units && !has_b; i++) {
             const CodedBitstreamUnit *unit = current->units + i;
-            if (unit->type <= VVC_RSV_IRAP_11) {
+            if (unit->content_ref && unit->type <= VVC_RSV_IRAP_11) {
                 const H266RawSliceHeader *rsh = unit->content_ref;
                 has_inter |= !IS_I(rsh);
                 has_b     |= IS_B(rsh);
-- 
2.48.1

_______________________________________________
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] [PATCH 2/2] avcodec/vvc/refs: Check content_ref in set_pict_type()

[Frank Plowman]

On 02/02/2025 21:17, Michael Niedermayer wrote:
> Fixes: 390565846/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-4990028521996288
> Fixes: Null pointer dereference
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/vvc/refs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
> index 486515d06db..1cfca482047 100644
> --- a/libavcodec/vvc/refs.c
> +++ b/libavcodec/vvc/refs.c
> @@ -186,7 +186,7 @@ static void set_pict_type(AVFrame *frame, const VVCContext *s, const VVCFrameCon
>          const CodedBitstreamFragment *current = &s->current_frame;
>          for (int i = 0; i < current->nb_units && !has_b; i++) {
>              const CodedBitstreamUnit *unit = current->units + i;
> -            if (unit->type <= VVC_RSV_IRAP_11) {
> +            if (unit->content_ref && unit->type <= VVC_RSV_IRAP_11) {
>                  const H266RawSliceHeader *rsh = unit->content_ref;
>                  has_inter |= !IS_I(rsh);
>                  has_b     |= IS_B(rsh);

I did a little more sniffing around this.  unit->content and
unit->content_ref are NULL for NAL units with a type code corresponding
with a reserved or unspecified NAL unit type.  Due to the existing
condition on the NAL unit type being a VCL NAL unit type, this means
that unit->type will be in [4..6], which are all reserved.

Perhaps we might want to add a warning message or something similar
letting the user know some data is being skipped, particularly seeing as
we are talking about video data here?  On the other hand, if the
loglevel is set to verbose or above, cbs_read_fragment_content will
produce some log output which eludes to this, although it is a bit
obtuse as codec-specific information is not available there.  In any
case, I agree that adding the extra check on unit->content_ref is correct.

Thank you,
Frank

_______________________________________________
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] [PATCH 2/2] avcodec/vvc/refs: Check content_ref in set_pict_type()

[Nuo Mi]

On Fri, Feb 7, 2025 at 4:25 AM Frank Plowman <post@frankplowman.com> wrote:

> On 02/02/2025 21:17, Michael Niedermayer wrote:
> > Fixes:
> 390565846/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-4990028521996288
> > Fixes: Null pointer dereference
> >
> > Found-by: continuous fuzzing process
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/vvc/refs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
> > index 486515d06db..1cfca482047 100644
> > --- a/libavcodec/vvc/refs.c
> > +++ b/libavcodec/vvc/refs.c
> > @@ -186,7 +186,7 @@ static void set_pict_type(AVFrame *frame, const
> VVCContext *s, const VVCFrameCon
> >          const CodedBitstreamFragment *current = &s->current_frame;
> >          for (int i = 0; i < current->nb_units && !has_b; i++) {
> >              const CodedBitstreamUnit *unit = current->units + i;
> > -            if (unit->type <= VVC_RSV_IRAP_11) {
> > +            if (unit->content_ref && unit->type <= VVC_RSV_IRAP_11) {
> >                  const H266RawSliceHeader *rsh = unit->content_ref;
> >                  has_inter |= !IS_I(rsh);
> >                  has_b     |= IS_B(rsh);
>
> I did a little more sniffing around this.  unit->content and
> unit->content_ref are NULL for NAL units with a type code corresponding
> with a reserved or unspecified NAL unit type.  Due to the existing
> condition on the NAL unit type being a VCL NAL unit type, this means
> that unit->type will be in [4..6], which are all reserved.
>
> Perhaps we might want to add a warning message or something similar
> letting the user know some data is being skipped, particularly seeing as
> we are talking about video data here?  On the other hand, if the
> loglevel is set to verbose or above, cbs_read_fragment_content will
> produce some log output which eludes to this, although it is a bit
> obtuse as codec-specific information is not available there.

We can do this with other patch.

> In any
> case, I agree that adding the extra check on unit->content_ref is correct.
>
Thank you, Frank and Micheal.
Will apply.

>
> Thank you,
> Frank
>
> _______________________________________________
> 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] [PATCH 1/2] avcodec/vvc/refs: fix negative pps_scaling_win offsets

[Frank Plowman]

On 02/02/2025 21:17, Michael Niedermayer wrote:
> The spec seems to allow these to  be negative
> 
> Fixes: left shift of negative value -15
> Fixes: 392687035/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-6559804532785152
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/vvc/refs.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
> index 8d4b7bb35b2..486515d06db 100644
> --- a/libavcodec/vvc/refs.c
> +++ b/libavcodec/vvc/refs.c
> @@ -147,10 +147,10 @@ static VVCFrame *alloc_frame(VVCContext *s, VVCFrameContext *fc)
>          for (int j = 0; j < frame->ctb_count; j++)
>              frame->rpl_tab[j] = frame->rpl;
>  
> -        win->left_offset   = pps->r->pps_scaling_win_left_offset   << sps->hshift[CHROMA];
> -        win->right_offset  = pps->r->pps_scaling_win_right_offset  << sps->hshift[CHROMA];
> -        win->top_offset    = pps->r->pps_scaling_win_top_offset    << sps->vshift[CHROMA];
> -        win->bottom_offset = pps->r->pps_scaling_win_bottom_offset << sps->vshift[CHROMA];
> +        win->left_offset   = pps->r->pps_scaling_win_left_offset   * (1 << sps->hshift[CHROMA]);
> +        win->right_offset  = pps->r->pps_scaling_win_right_offset  * (1 << sps->hshift[CHROMA]);
> +        win->top_offset    = pps->r->pps_scaling_win_top_offset    * (1 << sps->vshift[CHROMA]);
> +        win->bottom_offset = pps->r->pps_scaling_win_bottom_offset * (1 << sps->vshift[CHROMA]);
>          frame->ref_width   = pps->r->pps_pic_width_in_luma_samples  - win->left_offset   - win->right_offset;
>          frame->ref_height  = pps->r->pps_pic_height_in_luma_samples - win->bottom_offset - win->top_offset;
>  

This patch LGTM.

-- 
Frank

_______________________________________________
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] [PATCH 1/2] avcodec/vvc/refs: fix negative pps_scaling_win offsets

[Michael Niedermayer]

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

On Mon, Feb 03, 2025 at 08:05:19AM +0000, Frank Plowman wrote:
> On 02/02/2025 21:17, Michael Niedermayer wrote:
> > The spec seems to allow these to  be negative
> > 
> > Fixes: left shift of negative value -15
> > Fixes: 392687035/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-6559804532785152
> > 
> > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavcodec/vvc/refs.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/libavcodec/vvc/refs.c b/libavcodec/vvc/refs.c
> > index 8d4b7bb35b2..486515d06db 100644
> > --- a/libavcodec/vvc/refs.c
> > +++ b/libavcodec/vvc/refs.c
> > @@ -147,10 +147,10 @@ static VVCFrame *alloc_frame(VVCContext *s, VVCFrameContext *fc)
> >          for (int j = 0; j < frame->ctb_count; j++)
> >              frame->rpl_tab[j] = frame->rpl;
> >  
> > -        win->left_offset   = pps->r->pps_scaling_win_left_offset   << sps->hshift[CHROMA];
> > -        win->right_offset  = pps->r->pps_scaling_win_right_offset  << sps->hshift[CHROMA];
> > -        win->top_offset    = pps->r->pps_scaling_win_top_offset    << sps->vshift[CHROMA];
> > -        win->bottom_offset = pps->r->pps_scaling_win_bottom_offset << sps->vshift[CHROMA];
> > +        win->left_offset   = pps->r->pps_scaling_win_left_offset   * (1 << sps->hshift[CHROMA]);
> > +        win->right_offset  = pps->r->pps_scaling_win_right_offset  * (1 << sps->hshift[CHROMA]);
> > +        win->top_offset    = pps->r->pps_scaling_win_top_offset    * (1 << sps->vshift[CHROMA]);
> > +        win->bottom_offset = pps->r->pps_scaling_win_bottom_offset * (1 << sps->vshift[CHROMA]);
> >          frame->ref_width   = pps->r->pps_pic_width_in_luma_samples  - win->left_offset   - win->right_offset;
> >          frame->ref_height  = pps->r->pps_pic_height_in_luma_samples - win->bottom_offset - win->top_offset;
> >  
> 
> This patch LGTM.

will apply

thx

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

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell

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