linux-kernel - Re: [PATCH v3] media: v4l2-ctrls: add full AV1 profile validation in validate_av1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFyCYyOFFMrDetScx_8_VgRpCVyTq_O0PGn1hDt7+UwMygqeXw@mail.gmail.com>
Date: Thu, 23 Oct 2025 14:03:15 +0100
From: John Cox <jc@...esim.co.uk>
To: Hans Verkuil <hverkuil+cisco@...nel.org>
Cc: opensource india <opensource206@...il.com>, mchehab@...nel.org, hverkuil@...nel.org, 
	ribalda@...omium.org, laurent.pinchart@...asonboard.com, yunkec@...gle.com, 
	sakari.ailus@...ux.intel.com, james.cowgill@...ize.com, 
	Nicolas Dufresne <nicolas.dufresne@...labora.com>, linux-media@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] media: v4l2-ctrls: add full AV1 profile validation in validate_av1_sequence()

On Thu, 23 Oct 2025 at 11:44, Hans Verkuil <hverkuil+cisco@...nel.org> wrote:
>
> On 23/10/2025 12:32, opensource india wrote:
> > On Wed, Oct 22, 2025 at 12:44 PM Hans Verkuil <hverkuil+cisco@...nel.org> wrote:
> >>
> >> Hi Pavan,
> >>
> >> On 13/09/2025 12:52, Pavan Bobba wrote:
> >>> Complete the "TODO: PROFILES" by enforcing profile-specific and
> >>> monochrome constraints as defined by the AV1 specification
> >>> (Section 5.5.2, "Color config syntax").
> >>>
> >>> The validator now checks:
> >>>
> >>>  - Flags: reject any unknown bits set in sequence->flags
> >>>  - Profile range: only profiles 0..2 are valid
> >>>  - Profile 0: 8/10-bit only, subsampling must be 4:2:0 (sx=1, sy=1),
> >>>    monochrome allowed
> >>>  - Profile 1: 8/10-bit only, subsampling must be 4:4:4 (sx=0, sy=0),
> >>>    monochrome forbidden
> >>>  - Profile 2:
> >>>     * 8/10-bit: only 4:2:2 allowed (sx=1, sy=0)
> >>>     * 12-bit: 4:4:4 (sx=0, sy=0), 4:2:2 (sx=1, sy=0), or 4:2:0 (sx=1, sy=1)
> >>>       allowed
> >>>  - Monochrome path (all profiles except 1): forces subsampling_x=1,
> >>>    subsampling_y=1, separate_uv_delta_q=0
> >>>
> >>> These checks prevent userspace from providing invalid AV1 sequence
> >>> headers that would otherwise be accepted, leading to undefined driver
> >>> or hardware behavior.
> >>
> >> This patch was merged in our media-committers next branch, but I noticed that
> >> it now fails the v4l2-compliance test for the visl driver.
> >>
> >> The cause is that the new validation now fails with the default values for
> >> this control as set in std_init_compound().
> >>
> >> You can test this yourself by loading the visl driver and then running
> >> v4l2-compliance -d /dev/videoX -E --verbose
> >> (-E stops at the first error)
> >>
> >> Can you provide a patch to initialize this control with sane values?
> >>
> >> Apologies for not noticing this before: there are some issues with the automatic
> >> regression tests in our CI, so the tests weren't run.
> >>
> >> Regards,
> >>
> >>         Hans
> >>
> >
> > Hi Hans Verkuil,
> >
> > Thank you so much for the review.
> > yes, v4l2-compliance expected to fail indeed since it is sending
> > default values which, our newly added code rejects as per
> > specification
> >
> > when you say patch, you mean patch for v4l2-compliance tool with
> > proper values so that v4l2 core driver can accept?
>
> No, std_init_compound() in the kernel needs to be patched so the initial
> value of this control passes the new validation tests. The initial control
> values should always be sane.

Whilst that is a good principle it makes almost no sense in this
context. There is almost no chance that a given bitstream will decode
against a default sequence header and failing to set it explicitly is
going to be a mistake on the users part. It seems to me that it is
better to have something that is detectable as unset rather than
something that is valid but wrong.

I accept that it is the V4L2 way to require "valid" default values for
all supported ctrls, but it seems to me to be actively unhelpful for
things like SPS / VPS / Tile Group Entry where if not set correctly
from bits of the bitstream that the kernel doesn't get to see they
will break the stream decode.

I'm not going to argue this point but I felt that I wanted to make it.

Regards

John Cox
> Regards,
>
>         Hans
>