[PATCH] media: v4l2-ctrls: document expected validation scope

[PATCH] media: v4l2-ctrls: document expected validation scope

[Linus Walleij]

After discussion on the mailing list it became clear that these
validations are not expected to be exhaustive.

Link: https://lore.kernel.org/linux-media/20260629133209.GG3054459@killaraus.ideasonboard.com/
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
index b8698b85bd80..58184b6e386f 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
@@ -118,6 +118,18 @@ correct. This prevents the situation where only some of the controls
 were set/get. Only low-level errors (e. g. a failed i2c command) can
 still cause this situation.
 
+The validation of controls is not expected to be exhaustive, for example
+custom controls under ``V4L2_CTRL_CLASS_USER`` would become very voluminous.
+The expected behaviour is to reject settings that could:
+
+1. Pose stability issues, such as turning the hardware unresponsive requiring
+   a hard reset, such controls must be rejected.
+
+2. Pose security issues, such as compromising unrelated kernel-contained
+   data to userspace.
+
+Extended validation of control values should be performed in userspace.
+
 .. tabularcolumns:: |p{6.8cm}|p{4.0cm}|p{6.5cm}|
 
 .. c:type:: v4l2_ext_control

---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260701-v4l2-doc-c60195d38d00

Best regards,
--  
Linus Walleij <linusw@kernel.org>

Re: [PATCH] media: v4l2-ctrls: document expected validation scope

[Jacopo Mondi]

Hi Linus

On Wed, Jul 01, 2026 at 10:11:20AM +0200, Linus Walleij wrote:
> After discussion on the mailing list it became clear that these
> validations are not expected to be exhaustive.
>
> Link: https://lore.kernel.org/linux-media/20260629133209.GG3054459@killaraus.ideasonboard.com/

The above linked discussion is about parameters buffers, which can
indeed contain large sets of data.

I'm not sure the same applies to extended controls ?

> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
>  Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> index b8698b85bd80..58184b6e386f 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> @@ -118,6 +118,18 @@ correct. This prevents the situation where only some of the controls
>  were set/get. Only low-level errors (e. g. a failed i2c command) can
>  still cause this situation.
>
> +The validation of controls is not expected to be exhaustive, for example
> +custom controls under ``V4L2_CTRL_CLASS_USER`` would become very voluminous.
> +The expected behaviour is to reject settings that could:
> +
> +1. Pose stability issues, such as turning the hardware unresponsive requiring
> +   a hard reset, such controls must be rejected.
> +
> +2. Pose security issues, such as compromising unrelated kernel-contained
> +   data to userspace.
> +
> +Extended validation of control values should be performed in userspace.
> +
>  .. tabularcolumns:: |p{6.8cm}|p{4.0cm}|p{6.5cm}|
>
>  .. c:type:: v4l2_ext_control
>
> ---
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
> change-id: 20260701-v4l2-doc-c60195d38d00
>
> Best regards,
> --
> Linus Walleij <linusw@kernel.org>
>

Re: [PATCH] media: v4l2-ctrls: document expected validation scope

[Hans Verkuil]

On 01/07/2026 10:11, Linus Walleij wrote:
> After discussion on the mailing list it became clear that these
> validations are not expected to be exhaustive.
> 
> Link: https://lore.kernel.org/linux-media/20260629133209.GG3054459@killaraus.ideasonboard.com/
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
>  Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> index b8698b85bd80..58184b6e386f 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> @@ -118,6 +118,18 @@ correct. This prevents the situation where only some of the controls
>  were set/get. Only low-level errors (e. g. a failed i2c command) can
>  still cause this situation.
>  
> +The validation of controls is not expected to be exhaustive, for example
> +custom controls under ``V4L2_CTRL_CLASS_USER`` would become very voluminous.

CLASS_USER has nothing to do with this. I'd rephrase this as:

"...compound controls can be very complex."

> +The expected behaviour is to reject settings that could:
> +
> +1. Pose stability issues, such as turning the hardware unresponsive requiring
> +   a hard reset, such controls must be rejected.
> +
> +2. Pose security issues, such as compromising unrelated kernel-contained
> +   data to userspace.
> +
> +Extended validation of control values should be performed in userspace.
> +

I don't like this. If drivers do not validate controls well enough, and so allow
stability or security issues, then that's simply a bug. Ideally that check is done
when the controls are set, but this is not always possible. Sometimes this is only
possible when e.g. streaming is in progress.

And what "Extended validation" actually means is rather vague.

I would actually prefer to make some changes elsewhere in this file:

diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
index b8698b85bd80..d78328152b75 100644
--- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
+++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
@@ -72,12 +72,6 @@ partial array, all elements have to be set or retrieved. The total size
 is calculated as ``elems`` * ``elem_size``. These values can be obtained
 by calling :ref:`VIDIOC_QUERY_EXT_CTRL <VIDIOC_QUERYCTRL>`.

-To change the value of a set of controls applications initialize the
-``id``, ``size``, ``reserved2`` and ``value/value64/string/ptr`` fields
-of each struct :c:type:`v4l2_ext_control` and call
-the :ref:`VIDIOC_S_EXT_CTRLS <VIDIOC_G_EXT_CTRLS>` ioctl. The controls will only be set if *all*
-control values are valid.
-
 To check if a set of controls have correct values applications
 initialize the ``id``, ``size``, ``reserved2`` and
 ``value/value64/string/ptr`` fields of each struct
@@ -86,6 +80,15 @@ initialize the ``id``, ``size``, ``reserved2`` and
 values are automatically adjusted to a valid value or if an error is
 returned.

+To change the value of a set of controls applications initialize the
+``id``, ``size``, ``reserved2`` and ``value/value64/string/ptr`` fields
+of each struct :c:type:`v4l2_ext_control` and call
+the :ref:`VIDIOC_S_EXT_CTRLS <VIDIOC_G_EXT_CTRLS>` ioctl.
+:ref:`VIDIOC_S_EXT_CTRLS <VIDIOC_G_EXT_CTRLS>` will implicitly call
+:ref:`VIDIOC_TRY_EXT_CTRLS <VIDIOC_G_EXT_CTRLS>` first and return an
+error if that fails. So the controls will only be set if *all*
+control values are valid.
+
 When the ``id`` or ``which`` is invalid drivers return an ``EINVAL`` error
 code. When the value is out of bounds drivers can choose to take the
 closest valid value or return an ``ERANGE`` error code, whatever seems more

The main change here is that it explicitly states that S_EXT_CTRLS implies
a call to TRY_EXT_CTRLS is called. Something that was never clearly stated
before, but it's important to know this. Because of this change the TRY_EXT_CTRLS
and S_EXT_CTRLS paragraphs are swapped since S_EXT_CTRLS now refers to
TRY_EXT_CTRLS.

And the TRY_EXT_CTRLS documentation already says that: "It is up to the driver
whether wrong values are automatically adjusted to a valid value or if an error is
returned."

I just saw Jacopo's reply that the discussion was for parameter buffers, not
for extended controls, but the reasoning is the same: the driver must validate
at some point (ideally when the buffer is queued up so userspace gets the error
immediately), and ensure there are no security or stability issues. But if there
are, then that's just a driver bug.

Regards,

	Hans

>  .. tabularcolumns:: |p{6.8cm}|p{4.0cm}|p{6.5cm}|
>  
>  .. c:type:: v4l2_ext_control
> 
> ---
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
> change-id: 20260701-v4l2-doc-c60195d38d00
> 
> Best regards,
> --  
> Linus Walleij <linusw@kernel.org>
> 
> 

Re: [PATCH] media: v4l2-ctrls: document expected validation scope

[Linus Walleij]

Hi Hans,

thanks for clarifying!

On Wed, Jul 1, 2026 at 11:03 AM Hans Verkuil <hverkuil+cisco@kernel.org> wrote:

> The main change here is that it explicitly states that S_EXT_CTRLS implies
> a call to TRY_EXT_CTRLS is called. Something that was never clearly stated
> before, but it's important to know this. Because of this change the TRY_EXT_CTRLS
> and S_EXT_CTRLS paragraphs are swapped since S_EXT_CTRLS now refers to
> TRY_EXT_CTRLS.
>
> And the TRY_EXT_CTRLS documentation already says that: "It is up to the driver
> whether wrong values are automatically adjusted to a valid value or if an error is
> returned."

This works for me, I missed that because I'm a rookie at this.

For the above diff (I guess you want to send the patch? Else
I can make it):
Acked-by: Linus Walleij <linusw@kernel.org>

> I just saw Jacopo's reply that the discussion was for parameter buffers, not
> for extended controls, but the reasoning is the same: the driver must validate
> at some point (ideally when the buffer is queued up so userspace gets the error
> immediately), and ensure there are no security or stability issues. But if there
> are, then that's just a driver bug.

I get it, thanks!

Yours,
Linus Walleij

Re: [PATCH] media: v4l2-ctrls: document expected validation scope

[Hans Verkuil]

On 01/07/2026 12:59, Linus Walleij wrote:
> Hi Hans,
> 
> thanks for clarifying!
> 
> On Wed, Jul 1, 2026 at 11:03 AM Hans Verkuil <hverkuil+cisco@kernel.org> wrote:
> 
>> The main change here is that it explicitly states that S_EXT_CTRLS implies
>> a call to TRY_EXT_CTRLS is called. Something that was never clearly stated
>> before, but it's important to know this. Because of this change the TRY_EXT_CTRLS
>> and S_EXT_CTRLS paragraphs are swapped since S_EXT_CTRLS now refers to
>> TRY_EXT_CTRLS.
>>
>> And the TRY_EXT_CTRLS documentation already says that: "It is up to the driver
>> whether wrong values are automatically adjusted to a valid value or if an error is
>> returned."
> 
> This works for me, I missed that because I'm a rookie at this.

No problem!

> 
> For the above diff (I guess you want to send the patch? Else
> I can make it):
> Acked-by: Linus Walleij <linusw@kernel.org>

I posted a clean patch for this with your Acked-by.

> 
>> I just saw Jacopo's reply that the discussion was for parameter buffers, not
>> for extended controls, but the reasoning is the same: the driver must validate
>> at some point (ideally when the buffer is queued up so userspace gets the error
>> immediately), and ensure there are no security or stability issues. But if there
>> are, then that's just a driver bug.
> 
> I get it, thanks!

Regards,

	Hans

Re: [PATCH] media: v4l2-ctrls: document expected validation scope

[Vincenzo Frascino]

On 01/07/2026 09:11, Linus Walleij wrote:
> After discussion on the mailing list it became clear that these
> validations are not expected to be exhaustive.
> 
> Link: https://lore.kernel.org/linux-media/20260629133209.GG3054459@killaraus.ideasonboard.com/
> Signed-off-by: Linus Walleij <linusw@kernel.org>

Thank you Linus for posting this. With the suggested additions would have
certainly helped my understanding.

With this:
Acked-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

> ---
>  Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> index b8698b85bd80..58184b6e386f 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst
> @@ -118,6 +118,18 @@ correct. This prevents the situation where only some of the controls
>  were set/get. Only low-level errors (e. g. a failed i2c command) can
>  still cause this situation.
>  
> +The validation of controls is not expected to be exhaustive, for example
> +custom controls under ``V4L2_CTRL_CLASS_USER`` would become very voluminous.
> +The expected behaviour is to reject settings that could:
> +
> +1. Pose stability issues, such as turning the hardware unresponsive requiring
> +   a hard reset, such controls must be rejected.
> +
> +2. Pose security issues, such as compromising unrelated kernel-contained
> +   data to userspace.
> +
> +Extended validation of control values should be performed in userspace.
> +
>  .. tabularcolumns:: |p{6.8cm}|p{4.0cm}|p{6.5cm}|
>  
>  .. c:type:: v4l2_ext_control
> 
> ---
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
> change-id: 20260701-v4l2-doc-c60195d38d00
> 
> Best regards,
> --  
> Linus Walleij <linusw@kernel.org>
> 

-- 
Regards,
Vincenzo