Re: [PATCH v2] media: v4l2-ioctl: zero the ext control built for VIDIOC_{G,S}_CTRL

[Hans Verkuil <hverkuil+cisco@kernel.org>]

On 25/09/2026 10:02, Nick Rogers wrote:
> When a driver implements the extended control ioctls but has no control
> handler, v4l_g_ctrl() and v4l_s_ctrl() pass VIDIOC_G_CTRL and
> VIDIOC_S_CTRL on as a single struct v4l2_ext_control built on the stack.
> Only its id and value are set, and check_ext_ctrls() clears reserved[0]
> and reserved2[0]; the control's size and the rest of both structures are
> left uninitialized.
> 
> A driver that forwards the controls rather than handling them through
> the control framework sees that stack garbage. The virtio-media driver
> under review takes a nonzero size as a payload to copy from userspace,
> so VIDIOC_G_CTRL and VIDIOC_S_CTRL fail with -EINVAL through it whenever
> the stack is dirty. GStreamer's V4L2 encoders set their profile with
> VIDIOC_S_CTRL, and cannot negotiate against such a device.
> 
> Zero-initialize both structures.
> 
> Assisted-by: LLM
> Signed-off-by: Nick Rogers <nick@getfieldwork.ai>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
> Changes in v2:
> - Assisted-by: LLM, per Documentation/process/coding-assistants.rst
>   (Alexandre)
> - Collected Nicolas's Reviewed-by
> 
> v1: https://lore.kernel.org/all/20260923160936.33445-1-nick@getfieldwork.ai/
> 
> Alexandre asked whether drivers should fill the structure themselves.
> The core builds it and passes it down, and a driver can't tell a
> translated G_CTRL/S_CTRL from a real extended control call, so I think
> it's the core's to zero. He's right that virtio-media will meet kernels
> without this, though, so the driver now guards against it too:
> https://lore.kernel.org/all/20260925080140.44696-1-nick@getfieldwork.ai/
> 
> Found running the virtio-media v9 series [1] in a VMM with a host-side
> stateful encoder: GStreamer's v4l2h264enc fails to negotiate because
> VIDIOC_S_CTRL returns -EINVAL. Tested on 6.18 with that series applied:
> VIDIOC_G_CTRL and VIDIOC_S_CTRL now reach the device intact, and
> v4l2-compliance 1.30.1 reports the same results with and without this
> patch. Build-tested on media.git next (arm64, W=1, no new warnings).

This is the correct patch: these two struct need to be cleared in the
code. In fact, this needs a Fixes tag and a CC to stable. It's just a bug.
Never been caught since there are very few drivers that do not have a
control handler. The only driver without a control handler is uvc, and
that does the equivalent of QUERY_EXT_CTRL to determine if the control id
refers to a compound control (that uses the 'size' field) or not.

I verified that there are no other places in the kernel where v4l2_ext_control(s)
isn't cleared before use.

The virtio-media driver can try the same thing as uvc does: query the
control and check the flags field to see if it has a payload or not
(V4L2_CTRL_FLAG_HAS_PAYLOAD). That will work with any kernel.

In the meantime, this is just a plain bug fix.

Frankly, rather embarrassing. I'm pretty sure I wrote this, and I should have
known better...

Regards,

	Hans

> 
> [1] https://lore.kernel.org/all/20260917171921.2810550-1-briandaniels@google.com/
> 
>  drivers/media/v4l2-core/v4l2-ioctl.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 17ba1ae70..b7d248ab7 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2357,8 +2357,8 @@ static int v4l_g_ctrl(const struct v4l2_ioctl_ops *ops, struct file *file,
>  	struct video_device *vfd = video_devdata(file);
>  	struct v4l2_control *p = arg;
>  	struct v4l2_fh *vfh = file_to_v4l2_fh(file);
> -	struct v4l2_ext_controls ctrls;
> -	struct v4l2_ext_control ctrl;
> +	struct v4l2_ext_controls ctrls = {};
> +	struct v4l2_ext_control ctrl = {};
>  
>  	if (vfh && vfh->ctrl_handler)
>  		return v4l2_g_ctrl(vfh->ctrl_handler, p);
> @@ -2388,8 +2388,8 @@ static int v4l_s_ctrl(const struct v4l2_ioctl_ops *ops, struct file *file,
>  	struct video_device *vfd = video_devdata(file);
>  	struct v4l2_control *p = arg;
>  	struct v4l2_fh *vfh = file_to_v4l2_fh(file);
> -	struct v4l2_ext_controls ctrls;
> -	struct v4l2_ext_control ctrl;
> +	struct v4l2_ext_controls ctrls = {};
> +	struct v4l2_ext_control ctrl = {};
>  	int ret;
>  
>  	if (vfh && vfh->ctrl_handler)