mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nick Rogers <nick@getfieldwork.ai>
To: Brian Daniels <briandaniels@google.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	adelva@google.com, aesteve@redhat.com, changyeon@google.com,
	daniel.almeida@collabora.com, eperezma@redhat.com,
	Alexandre Courbot <gnurou@gmail.com>,
	gurchetansingh@google.com, Hans Verkuil <hverkuil@xs4all.nl>,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	mst@redhat.com, Nicolas Dufresne <nicolas.dufresne@collabora.com>,
	virtualization@lists.linux.dev, xuanzhuo@linux.alibaba.com,
	dbassey@redhat.com, laurent.pinchart@ideasonboard.com
Subject: [PATCH] media: virtio: clear the size of legacy controls built on the stack
Date: Fri, 25 Sep 2026 09:01:40 +0100	[thread overview]
Message-ID: <20260925080140.44696-1-nick@getfieldwork.ai> (raw)
In-Reply-To: <20260923162928.73497-1-nick@getfieldwork.ai>

A driver without a control handler receives VIDIOC_G_CTRL and
VIDIOC_S_CTRL from the V4L2 core as a single extended control that the
core builds on its own stack, with the control's size left
uninitialized. virtio_media_send_ext_controls_ioctl() takes a nonzero
size as a payload to copy from userspace, so these ioctls fail with
-EINVAL whenever the stack is dirty. GStreamer's V4L2 encoders set their
profile with VIDIOC_S_CTRL and cannot negotiate, and v4l2-ctl cannot
read the MIN_BUFFERS controls.

Legacy controls carry a 32-bit value and never a payload, and an array
that comes from userspace is copied into an allocation, never onto the
stack. Clear the size of each control when the array is on the stack.

The core is being fixed to zero the structure [1]; this keeps the driver
working on kernels without that fix.

[1] https://lore.kernel.org/all/20260923160936.33445-1-nick@getfieldwork.ai/

Assisted-by: LLM
Signed-off-by: Nick Rogers <nick@getfieldwork.ai>
---
This applies on top of the virtio-media v9 series and turns point 3 of
my review above into a patch. Build-tested on media.git next with v9
applied (arm64, W=1, no new warnings).

 drivers/media/virtio/virtio_media_ioctls.c | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/media/virtio/virtio_media_ioctls.c b/drivers/media/virtio/virtio_media_ioctls.c
index f0b82b5ec..19abcf95e 100644
--- a/drivers/media/virtio/virtio_media_ioctls.c
+++ b/drivers/media/virtio/virtio_media_ioctls.c
@@ -6,6 +6,7 @@
  * Copyright (c) 2024-2026 Google LLC.
  */
 
+#include <linux/sched/task_stack.h>
 #include <linux/mutex.h>
 #include <linux/videodev2.h>
 #include <linux/virtio_config.h>
@@ -576,11 +577,31 @@ static int virtio_media_querycap(struct file *file, void *fh,
  * Extended control ioctls are handled mostly identically.
  */
 
+/*
+ * VIDIOC_G_CTRL and VIDIOC_S_CTRL reach a driver without a control handler
+ * as one extended control the V4L2 core builds on its own stack, with its
+ * size left uninitialized. They carry a 32-bit value and never a payload,
+ * so a size there is stack garbage, which would be sent as a user pointer
+ * to copy and fail the ioctl. An array from userspace is never on the
+ * stack: the core copies it into an allocation.
+ */
+static void virtio_media_legacy_ctrl(struct v4l2_ext_controls *ctrls)
+{
+	u32 i;
+
+	if (!ctrls->count || !object_is_on_stack(ctrls->controls))
+		return;
+	for (i = 0; i < ctrls->count; i++)
+		ctrls->controls[i].size = 0;
+}
+
 static int virtio_media_g_ext_ctrls(struct file *file, void *fh,
 				    struct v4l2_ext_controls *ctrls)
 {
 	struct v4l2_fh *vfh = file_to_v4l2_fh(file);
 
+	virtio_media_legacy_ctrl(ctrls);
+
 	return virtio_media_send_ext_controls_ioctl(vfh, VIDIOC_G_EXT_CTRLS,
 						    ctrls);
 }
@@ -590,6 +611,8 @@ static int virtio_media_s_ext_ctrls(struct file *file, void *fh,
 {
 	struct v4l2_fh *vfh = file_to_v4l2_fh(file);
 
+	virtio_media_legacy_ctrl(ctrls);
+
 	return virtio_media_send_ext_controls_ioctl(vfh, VIDIOC_S_EXT_CTRLS,
 						    ctrls);
 }
@@ -599,6 +622,8 @@ static int virtio_media_try_ext_ctrls(struct file *file, void *fh,
 {
 	struct v4l2_fh *vfh = file_to_v4l2_fh(file);
 
+	virtio_media_legacy_ctrl(ctrls);
+
 	return virtio_media_send_ext_controls_ioctl(vfh, VIDIOC_TRY_EXT_CTRLS,
 						    ctrls);
 }
-- 
2.54.0 (Apple Git-157)


      reply	other threads:[~2026-09-25  8:01 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17 17:19 [PATCH v9 0/4] media: add virtio-media driver Brian Daniels
2026-09-17 17:19 ` [PATCH v9 1/4] media: virtio: Add skeleton " Brian Daniels
2026-09-18 15:10   ` Albert Esteve
2026-09-22 18:33     ` Brian Daniels
2026-09-19 19:52   ` Michael S. Tsirkin
2026-09-22 18:36     ` Brian Daniels
2026-09-19 20:03   ` Michael S. Tsirkin
2026-09-22 18:37     ` Brian Daniels
2026-09-17 17:19 ` [PATCH v9 2/4] media: virtio: Add session management Brian Daniels
2026-09-18 15:12   ` Albert Esteve
2026-09-19 20:00     ` Michael S. Tsirkin
2026-09-17 17:19 ` [PATCH v9 3/4] media: virtio: Add scatterlist builder Brian Daniels
2026-09-19 20:01   ` Michael S. Tsirkin
2026-09-17 17:19 ` [PATCH v9 4/4] media: virtio: Add ioctl operations and driver logic Brian Daniels
2026-09-18 15:15   ` Albert Esteve
2026-09-23 16:29   ` Nick Rogers
2026-09-25  8:01     ` Nick Rogers [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260925080140.44696-1-nick@getfieldwork.ai \
    --to=nick@getfieldwork.ai \
    --cc=adelva@google.com \
    --cc=aesteve@redhat.com \
    --cc=briandaniels@google.com \
    --cc=changyeon@google.com \
    --cc=daniel.almeida@collabora.com \
    --cc=dbassey@redhat.com \
    --cc=eperezma@redhat.com \
    --cc=gnurou@gmail.com \
    --cc=gurchetansingh@google.com \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=mst@redhat.com \
    --cc=nicolas.dufresne@collabora.com \
    --cc=virtualization@lists.linux.dev \
    --cc=xuanzhuo@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®