From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
Hans Verkuil <hverkuil+cisco@kernel.org>,
Sasha Levin <sashal@kernel.org>,
awalls@md.metrocast.net, linux-media@vger.kernel.org
Subject: [PATCH AUTOSEL 6.17-5.4] media: pci: ivtv: Don't create fake v4l2_fh
Date: Sat, 25 Oct 2025 11:58:26 -0400 [thread overview]
Message-ID: <20251025160905.3857885-275-sashal@kernel.org> (raw)
In-Reply-To: <20251025160905.3857885-1-sashal@kernel.org>
From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
[ Upstream commit cc6e8d1ccea792d8550428e0831e3a35b0ccfddc ]
The ivtv driver has a structure named ivtv_open_id that models an open
file handle for the device. It embeds a v4l2_fh instance for file
handles that correspond to a V4L2 video device, and stores a pointer to
that v4l2_fh in struct ivtv_stream to identify which open file handle
owns a particular stream.
In addition to video devices, streams can be owned by ALSA PCM devices.
Those devices do not make use of the v4l2_fh instance for obvious
reasons, but the snd_ivtv_pcm_capture_open() function still initializes
a "fake" v4l2_fh for the sole purpose of using it as an open file handle
identifier. The v4l2_fh is not properly destroyed when the ALSA PCM
device is closed, leading to possible resource leaks.
Fortunately, the v4l2_fh instance pointed to by ivtv_stream is not
accessed, only the pointer value is used for comparison. Replace it with
a pointer to the ivtv_open_id structure that embeds the v4l2_fh, and
don't initialize the v4l2_fh for ALSA PCM devices.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.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 is a good stable backport
- Fixes a real bug: ALSA PCM open in ivtv initialized a fake v4l2_fh
that was never properly destroyed on success or on “already streaming”
early return, causing a kernel memory leak across repeated open/close
cycles.
- Small, contained change: confines to ivtv driver internals (media:
pci: ivtv), with no API/ABI changes to V4L2/ALSA frameworks.
- Behavior-preserving: the driver used the v4l2_fh pointer solely as an
opaque owner identifier (pointer equality/non-NULL), never
dereferencing it. Switching to an ivtv_open_id pointer preserves
semantics without allocating any v4l2_fh resources.
- Low regression risk: only owner-tracking logic and ALSA open path are
touched; no architectural changes; no critical core subsystems
affected.
- User impact: prevents kernel memory leakage reachable by regular users
via the ALSA PCM device node.
What the patch changes (with code evidence)
- Stop creating a fake v4l2_fh for ALSA PCM open:
- Removes `v4l2_fh_init(&item.fh, &s->vdev)` in ALSA open, avoiding
allocation that was never exited in the success/streaming path.
- Evidence: drivers/media/pci/ivtv/ivtv-alsa-pcm.c:151
- Switch stream owner tracking from v4l2_fh* to ivtv_open_id*:
- Struct change: `struct ivtv_stream` replaces `struct v4l2_fh *fh;`
with `struct ivtv_open_id *id;`
- Evidence: drivers/media/pci/ivtv/ivtv-driver.h:334
- Adds forward decl for `struct ivtv_open_id;` so the pointer type can
be used earlier in the header (part of the change).
- Update owner comparisons and checks accordingly:
- ivtv_claim_stream now compares `s->id == id` instead of `s->fh ==
&id->fh`, assigns `s->id = id`, and handles VBI special-case the
same way.
- Evidence: drivers/media/pci/ivtv/ivtv-fileops.c:42, 46–51, 59
- ivtv_release_stream clears `s->id = NULL` (was `s->fh = NULL`) and
uses `s_vbi->id` for “still claimed” checks.
- Evidence: drivers/media/pci/ivtv/ivtv-fileops.c:97, 129
- ivtv_read initialization check uses `s->id == NULL` instead of
`s->fh == NULL`.
- Evidence: drivers/media/pci/ivtv/ivtv-fileops.c:362
- ivtv_stop_capture VBI-internal use case sets `s->id = NULL` instead
of `s->fh = NULL`.
- Evidence: drivers/media/pci/ivtv/ivtv-fileops.c:834
- ivtv_v4l2_close compares `s->id != id` instead of `s->fh != &id->fh`
to detect ownership.
- Evidence: drivers/media/pci/ivtv/ivtv-fileops.c:918
- ivtv-irq VBI data handling checks `s->id == NULL` (was `s->fh ==
NULL`) to decide whether to free buffers if no owner, and uses
`s->id` for wakeups.
- Evidence: drivers/media/pci/ivtv/ivtv-irq.c:301–312 (check), 333–334
(wake_up)
- Remove the now-unneeded v4l2_fh exit on the ALSA open fail path:
- Since we no longer init a fake fh, there’s nothing to exit on
-EBUSY.
- Evidence: drivers/media/pci/ivtv/ivtv-alsa-pcm.c:151 (removal
implies corresponding exit removal)
Why this fixes the leak
- Before: `snd_ivtv_pcm_capture_open()` created `item.fh` via
`v4l2_fh_init` and then:
- On successful claim and subsequent returns (including the “already
streaming” fast path), there was no matching `v4l2_fh_exit`, leaking
internal resources of `v4l2_fh`. See initialization at
drivers/media/pci/ivtv/ivtv-alsa-pcm.c:151 and early return in the
streaming case just after claim.
- After: There is no `v4l2_fh_init` for ALSA opens; the driver tracks
ownership with a raw `ivtv_open_id *` pointer whose address is used
only as a token (never dereferenced), eliminating the need for any
initialization or teardown on the ALSA path.
Risk and side effects
- Preserved semantics: s->fh was never dereferenced; it was used only
for pointer equality and non-NULL checks. Replacing with s->id keeps
those semantics while removing the artificial fh lifecycle.
- Internal-only: Changes are contained within ivtv; no external APIs
affected. Struct layout changed only internally to the driver.
- Concurrency/logic continuity: The VBI special-case claim/release and
wakeup conditions are updated consistently to use s->id; read path and
IRQ paths keep identical logic, just different pointer type.
Stable criteria check
- Important bugfix: addresses a user-visible kernel memory leak.
- Minimal, localized change: touches only ivtv, no cross-subsystem
churn.
- No new features or architectural changes.
- Low regression risk, with clear correctness rationale.
- No explicit “Cc: stable” tag in the message, but it meets the stable
rules for critical bug fixes with minimal risk.
Backport notes
- Ensure all s->fh uses in ivtv are converted to s->id. In the current
tree, these occur at:
- drivers/media/pci/ivtv/ivtv-fileops.c:42, 46–51, 59, 97, 129, 362,
834, 918
- drivers/media/pci/ivtv/ivtv-irq.c:308, 333
- drivers/media/pci/ivtv/ivtv-driver.h:334 (field)
- No other ivtv files refer to s->fh; remaining fh usage is via id->fh
for V4L2 event/poll mechanisms and is unchanged.
- The forward declaration `struct ivtv_open_id;` must be added before
`struct ivtv_stream` in ivtv-driver.h to avoid compile errors.
Conclusion
- This commit fixes a resource leak in a safe, contained way without
changing behavior, and is suitable for backporting to stable kernel
trees.
drivers/media/pci/ivtv/ivtv-alsa-pcm.c | 2 --
drivers/media/pci/ivtv/ivtv-driver.h | 3 ++-
drivers/media/pci/ivtv/ivtv-fileops.c | 18 +++++++++---------
drivers/media/pci/ivtv/ivtv-irq.c | 4 ++--
4 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c
index 8f346d7da9c8d..269a799ec046c 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c
@@ -148,14 +148,12 @@ static int snd_ivtv_pcm_capture_open(struct snd_pcm_substream *substream)
s = &itv->streams[IVTV_ENC_STREAM_TYPE_PCM];
- v4l2_fh_init(&item.fh, &s->vdev);
item.itv = itv;
item.type = s->type;
/* See if the stream is available */
if (ivtv_claim_stream(&item, item.type)) {
/* No, it's already in use */
- v4l2_fh_exit(&item.fh);
snd_ivtv_unlock(itvsc);
return -EBUSY;
}
diff --git a/drivers/media/pci/ivtv/ivtv-driver.h b/drivers/media/pci/ivtv/ivtv-driver.h
index a6ffa99e16bc6..83818048f7fe4 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.h
+++ b/drivers/media/pci/ivtv/ivtv-driver.h
@@ -322,6 +322,7 @@ struct ivtv_queue {
};
struct ivtv; /* forward reference */
+struct ivtv_open_id;
struct ivtv_stream {
/* These first four fields are always set, even if the stream
@@ -331,7 +332,7 @@ struct ivtv_stream {
const char *name; /* name of the stream */
int type; /* stream type */
- struct v4l2_fh *fh; /* pointer to the streaming filehandle */
+ struct ivtv_open_id *id; /* pointer to the streaming ivtv_open_id */
spinlock_t qlock; /* locks access to the queues */
unsigned long s_flags; /* status flags, see above */
int dma; /* can be PCI_DMA_TODEVICE, PCI_DMA_FROMDEVICE or PCI_DMA_NONE */
diff --git a/drivers/media/pci/ivtv/ivtv-fileops.c b/drivers/media/pci/ivtv/ivtv-fileops.c
index cfa28d0355863..1ac8d691df5cd 100644
--- a/drivers/media/pci/ivtv/ivtv-fileops.c
+++ b/drivers/media/pci/ivtv/ivtv-fileops.c
@@ -39,16 +39,16 @@ int ivtv_claim_stream(struct ivtv_open_id *id, int type)
if (test_and_set_bit(IVTV_F_S_CLAIMED, &s->s_flags)) {
/* someone already claimed this stream */
- if (s->fh == &id->fh) {
+ if (s->id == id) {
/* yes, this file descriptor did. So that's OK. */
return 0;
}
- if (s->fh == NULL && (type == IVTV_DEC_STREAM_TYPE_VBI ||
+ if (s->id == NULL && (type == IVTV_DEC_STREAM_TYPE_VBI ||
type == IVTV_ENC_STREAM_TYPE_VBI)) {
/* VBI is handled already internally, now also assign
the file descriptor to this stream for external
reading of the stream. */
- s->fh = &id->fh;
+ s->id = id;
IVTV_DEBUG_INFO("Start Read VBI\n");
return 0;
}
@@ -56,7 +56,7 @@ int ivtv_claim_stream(struct ivtv_open_id *id, int type)
IVTV_DEBUG_INFO("Stream %d is busy\n", type);
return -EBUSY;
}
- s->fh = &id->fh;
+ s->id = id;
if (type == IVTV_DEC_STREAM_TYPE_VBI) {
/* Enable reinsertion interrupt */
ivtv_clear_irq_mask(itv, IVTV_IRQ_DEC_VBI_RE_INSERT);
@@ -94,7 +94,7 @@ void ivtv_release_stream(struct ivtv_stream *s)
struct ivtv *itv = s->itv;
struct ivtv_stream *s_vbi;
- s->fh = NULL;
+ s->id = NULL;
if ((s->type == IVTV_DEC_STREAM_TYPE_VBI || s->type == IVTV_ENC_STREAM_TYPE_VBI) &&
test_bit(IVTV_F_S_INTERNAL_USE, &s->s_flags)) {
/* this stream is still in use internally */
@@ -126,7 +126,7 @@ void ivtv_release_stream(struct ivtv_stream *s)
/* was already cleared */
return;
}
- if (s_vbi->fh) {
+ if (s_vbi->id) {
/* VBI stream still claimed by a file descriptor */
return;
}
@@ -359,7 +359,7 @@ static ssize_t ivtv_read(struct ivtv_stream *s, char __user *ubuf, size_t tot_co
size_t tot_written = 0;
int single_frame = 0;
- if (atomic_read(&itv->capturing) == 0 && s->fh == NULL) {
+ if (atomic_read(&itv->capturing) == 0 && s->id == NULL) {
/* shouldn't happen */
IVTV_DEBUG_WARN("Stream %s not initialized before read\n", s->name);
return -EIO;
@@ -831,7 +831,7 @@ void ivtv_stop_capture(struct ivtv_open_id *id, int gop_end)
id->type == IVTV_ENC_STREAM_TYPE_VBI) &&
test_bit(IVTV_F_S_INTERNAL_USE, &s->s_flags)) {
/* Also used internally, don't stop capturing */
- s->fh = NULL;
+ s->id = NULL;
}
else {
ivtv_stop_v4l2_encode_stream(s, gop_end);
@@ -915,7 +915,7 @@ int ivtv_v4l2_close(struct file *filp)
v4l2_fh_exit(fh);
/* Easy case first: this stream was never claimed by us */
- if (s->fh != &id->fh)
+ if (s->id != id)
goto close_done;
/* 'Unclaim' this stream */
diff --git a/drivers/media/pci/ivtv/ivtv-irq.c b/drivers/media/pci/ivtv/ivtv-irq.c
index 4d63daa01eed2..078d9cd77c710 100644
--- a/drivers/media/pci/ivtv/ivtv-irq.c
+++ b/drivers/media/pci/ivtv/ivtv-irq.c
@@ -305,7 +305,7 @@ static void dma_post(struct ivtv_stream *s)
ivtv_process_vbi_data(itv, buf, 0, s->type);
s->q_dma.bytesused += buf->bytesused;
}
- if (s->fh == NULL) {
+ if (s->id == NULL) {
ivtv_queue_move(s, &s->q_dma, NULL, &s->q_free, 0);
return;
}
@@ -330,7 +330,7 @@ static void dma_post(struct ivtv_stream *s)
set_bit(IVTV_F_I_HAVE_WORK, &itv->i_flags);
}
- if (s->fh)
+ if (s->id)
wake_up(&s->waitq);
}
--
2.51.0
next prev parent reply other threads:[~2025-10-25 16:21 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 ` Sasha Levin [this message]
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 ` [PATCH AUTOSEL 6.17-6.1] media: verisilicon: Explicitly disable selection api ioctls for decoders Sasha Levin
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-275-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=awalls@md.metrocast.net \
--cc=hverkuil+cisco@kernel.org \
--cc=laurent.pinchart+renesas@ideasonboard.com \
--cc=linux-media@vger.kernel.org \
--cc=patches@lists.linux.dev \
--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