Linux Media Controller development
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Paul Kocialkowski <paulk@sys-base.io>,
	Nicolas Dufresne <nicolas.dufresne@collabora.com>,
	Hans Verkuil <hverkuil+cisco@kernel.org>,
	Sasha Levin <sashal@kernel.org>,
	benjamin.gaignard@collabora.com, p.zabel@pengutronix.de,
	linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org
Subject: [PATCH AUTOSEL 6.17-6.1] media: verisilicon: Explicitly disable selection api ioctls for decoders
Date: Sat, 25 Oct 2025 11:59:03 -0400	[thread overview]
Message-ID: <20251025160905.3857885-312-sashal@kernel.org> (raw)
In-Reply-To: <20251025160905.3857885-1-sashal@kernel.org>

From: Paul Kocialkowski <paulk@sys-base.io>

[ Upstream commit 73d50aa92f28ee8414fbfde011974fce970b82cc ]

Call the dedicated v4l2_disable_ioctl helper instead of manually
checking whether the current context is an encoder for the selection
api ioctls.

Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

YES

**Why This Fix Matters**
- Correctly hides unsupported ioctls on decoder nodes: Previously, the
  driver exposed `VIDIOC_G_SELECTION`/`VIDIOC_S_SELECTION` to decoders
  but rejected them at runtime with `-EINVAL`. This incorrectly
  advertised capability and confused userspace and core heuristics. With
  this change, those ioctls are explicitly disabled and return
  `-ENOTTY`, which is the correct “not supported” error and matches V4L2
  expectations (drivers/media/v4l2-core/v4l2-ioctl.c:3073, 3111).
- Avoids misleading legacy crop exposure: V4L2 core auto-enables legacy
  crop ioctls if selection is available. Disabling selection for
  decoders prevents the core from enabling `VIDIOC_G_CROP/CROPCAP` on
  decoder nodes (drivers/media/v4l2-core/v4l2-dev.c:657,
  drivers/media/v4l2-core/v4l2-dev.c:659,
  drivers/media/v4l2-core/v4l2-dev.c:662,
  drivers/media/v4l2-core/v4l2-dev.c:663,
  drivers/media/v4l2-core/v4l2-dev.c:664,
  drivers/media/v4l2-core/v4l2-dev.c:665). This fixes a user-visible API
  correctness issue.

**Change Details**
- Disables selection ioctls for decoder device nodes using the standard
  helper, before registration:
  - `v4l2_disable_ioctl(vfd, VIDIOC_G_SELECTION);`
  - `v4l2_disable_ioctl(vfd, VIDIOC_S_SELECTION);`
  - Location: drivers/media/platform/verisilicon/hantro_drv.c:918,
    drivers/media/platform/verisilicon/hantro_drv.c:919
  - Called before `video_register_device`, as required
    (drivers/media/platform/verisilicon/hantro_drv.c:924).
- Simplifies selection handlers to only enforce buffer type, removing
  runtime checks on context role:
  - Dropped `!ctx->is_encoder` checks; now only `sel->type !=
    V4L2_BUF_TYPE_VIDEO_OUTPUT` is validated.
  - `vidioc_g_selection`:
    drivers/media/platform/verisilicon/hantro_v4l2.c:666–667
  - `vidioc_s_selection`:
    drivers/media/platform/verisilicon/hantro_v4l2.c:698–699
  - Effect: No functional change for encoders (where `ctx->is_encoder`
    is always true), and decoders won’t reach these handlers since the
    ioctls are disabled.

**Risk and Side Effects**
- Behavior change is limited to decoders for selection ioctls: return
  code changes from `-EINVAL` to `-ENOTTY` via core gating
  (`is_valid_ioctl()` fails, `ret` remains `-ENOTTY`,
  drivers/media/v4l2-core/v4l2-ioctl.c:3073, 3111–3113). This is the
  correct semantics for “unsupported ioctl” and improves userspace
  detection.
- No architectural changes; confined to the Verisilicon Hantro driver.
  Encoder behavior is unchanged.
- Very small, contained patch; unlikely to introduce regressions. Aligns
  with common media driver practice of disabling non-applicable ioctls
  for a given node.

**Stable Backport Fit**
- Fixes a user-visible API bug (misadvertised capability and wrong
  errno) with minimal, localized changes.
- No new features or interfaces; follows stable rules for correctness
  fixes.
- Touches a non-core subsystem (media, platform driver), minimizing
  cross-subsystem risk.

Given the above, this commit is a good candidate for stable backporting.

 drivers/media/platform/verisilicon/hantro_drv.c  | 2 ++
 drivers/media/platform/verisilicon/hantro_v4l2.c | 6 ++----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
index fa972effd4a2c..9d5e50fedae1f 100644
--- a/drivers/media/platform/verisilicon/hantro_drv.c
+++ b/drivers/media/platform/verisilicon/hantro_drv.c
@@ -917,6 +917,8 @@ static int hantro_add_func(struct hantro_dev *vpu, unsigned int funcid)
 		vpu->decoder = func;
 		v4l2_disable_ioctl(vfd, VIDIOC_TRY_ENCODER_CMD);
 		v4l2_disable_ioctl(vfd, VIDIOC_ENCODER_CMD);
+		v4l2_disable_ioctl(vfd, VIDIOC_G_SELECTION);
+		v4l2_disable_ioctl(vfd, VIDIOC_S_SELECTION);
 	}
 
 	video_set_drvdata(vfd, vpu);
diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index 7c3515cf7d64a..4598f9b4bd21c 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -663,8 +663,7 @@ static int vidioc_g_selection(struct file *file, void *priv,
 	struct hantro_ctx *ctx = fh_to_ctx(priv);
 
 	/* Crop only supported on source. */
-	if (!ctx->is_encoder ||
-	    sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+	if (sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
 		return -EINVAL;
 
 	switch (sel->target) {
@@ -696,8 +695,7 @@ static int vidioc_s_selection(struct file *file, void *priv,
 	struct vb2_queue *vq;
 
 	/* Crop only supported on source. */
-	if (!ctx->is_encoder ||
-	    sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+	if (sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
 		return -EINVAL;
 
 	/* Change not allowed if the queue is streaming. */
-- 
2.51.0


  parent reply	other threads:[~2025-10-25 16:23 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20251025160905.3857885-1-sashal@kernel.org>
2025-10-25 15:53 ` [PATCH AUTOSEL 6.17] media: nxp: imx8-isi: Fix streaming cleanup on release Sasha Levin
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17] drm/xe: improve dma-resv handling for backup object Sasha Levin
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17-5.15] media: adv7180: Only validate format in querystd Sasha Levin
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17-6.6] media: ov08x40: Fix the horizontal flip control Sasha Levin
2025-10-25 15:55 ` [PATCH AUTOSEL 6.17-5.15] media: em28xx: add special case for legacy gpiolib interface Sasha Levin
2025-10-27  9:24   ` Arnd Bergmann
2025-11-04 13:55     ` Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17-5.15] media: adv7180: Do not write format to device in set_fmt Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17] media: imx-mipi-csis: Only set clock rate when specified in DT Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17] media: qcom: camss: csiphy-3ph: Add CSIPHY 2ph DPHY v2.0.1 init sequence Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17-5.4] media: redrat3: use int type to store negative error codes Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17-5.4] media: pci: ivtv: Don't create fake v4l2_fh Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17-5.4] media: imon: make send_packet() more robust Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17-5.15] media: adv7180: Add missing lock in suspend callback Sasha Levin
2025-10-25 15:59 ` Sasha Levin [this message]
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17-6.12] media: pci: mgb4: Fix timings comparison in VIDIOC_S_DV_TIMINGS Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17-6.12] media: ipu6: isys: Set embedded data type correctly for metadata formats Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17-6.1] media: i2c: og01a1b: Specify monochrome media bus format instead of Bayer Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17-6.1] media: amphion: Delete v4l2_fh synchronously in .release() Sasha Levin

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=20251025160905.3857885-312-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=benjamin.gaignard@collabora.com \
    --cc=hverkuil+cisco@kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=nicolas.dufresne@collabora.com \
    --cc=p.zabel@pengutronix.de \
    --cc=patches@lists.linux.dev \
    --cc=paulk@sys-base.io \
    --cc=stable@vger.kernel.org \
    /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