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

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

[Nick Rogers]

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: Claude:claude-opus-5-5
Signed-off-by: Nick Rogers <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).

[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)
-- 
2.54.0 (Apple Git-157)

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

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 3046 bytes --]

Hi,

Le mercredi 23 septembre 2026 à 17:09 +0100, Nick Rogers a écrit :
> 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: Claude:claude-opus-5-5
> Signed-off-by: Nick Rogers <nick@getfieldwork.ai>

One may argue that it could have been fixed in the unique driver not using the
ctrl framework, but I personally like the simplicity of the fix.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Alexandre Courbot]

On Thu Sep 24, 2026 at 1:09 AM JST, 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: Claude:claude-opus-5-5
> Signed-off-by: Nick Rogers <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).
>
> [1] https://lore.kernel.org/all/20260917171921.2810550-1-briandaniels@google.com/

I'm wondering what the API contract intent is here. If it is that
drivers are supposed to fill the control structures themselves, then
virtio-media should also be fixed, regardless of this safeguard.

Indeed it is likely that virtio-media will be ported to some older
kernels, which may not have this patch, in which case the same bug will
arise again.

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

[Alexandre Courbot]

On Thu Sep 24, 2026 at 1:09 AM JST, 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: Claude:claude-opus-5-5

nit: per the kernel guidelines [1], this should be

  Assisted-by: LLM

[1] https://docs.kernel.org/process/coding-assistants.html#attribution