[PATCH v4 0/2] media: pci: Fix invalid access to file *

[PATCH v4 0/2] media: pci: Fix invalid access to file *

[Jacopo Mondi]

Since commits
7b9eb53e8591 ("media: cx18: Access v4l2_fh from file")
9ba9d11544f9 ("media: ivtv: Access v4l2_fh from file")

All the ioctl handlers access their private data structures
from file *

The ivtv and cx18 drivers call the ioctl handlers from their
DVB layer without a valid file *, causing invalid memory access.

The issue has been reported by smatch in
"[bug report] media: cx18: Access v4l2_fh from file"

Fix this by providing wrappers for the ioctl handlers to be
used by the DVB layer that do not require a valid file *.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
Changes in v4:
- Slightly adjust commit messages
- Link to v3: https://lore.kernel.org/r/20250818-cx18-v4l2-fh-v3-0-5e2f08f3cadc@ideasonboard.com

Changes in v3:
- Change helpers to accept the type they're going to operate on instead
  of using the open_id wrapper type as suggested by Laurent
- Link to v2: https://lore.kernel.org/r/20250818-cx18-v4l2-fh-v2-0-3f53ce423663@ideasonboard.com

Changes in v2:
- Add Cc: stable@vger.kernel.org per-patch

---
Jacopo Mondi (2):
      media: cx18: Fix invalid access to file *
      media: ivtv: Fix invalid access to file *

 drivers/media/pci/cx18/cx18-driver.c |  9 +++------
 drivers/media/pci/cx18/cx18-ioctl.c  | 30 +++++++++++++++++++-----------
 drivers/media/pci/cx18/cx18-ioctl.h  |  8 +++++---
 drivers/media/pci/ivtv/ivtv-driver.c | 11 ++++-------
 drivers/media/pci/ivtv/ivtv-ioctl.c  | 22 +++++++++++++++++-----
 drivers/media/pci/ivtv/ivtv-ioctl.h  |  6 ++++--
 6 files changed, 52 insertions(+), 34 deletions(-)
---
base-commit: a75b8d198c55e9eb5feb6f6e155496305caba2dc
change-id: 20250818-cx18-v4l2-fh-7eaa6199fdde

Best regards,
-- 
Jacopo Mondi <jacopo.mondi@ideasonboard.com>

[PATCH v4 1/2] media: cx18: Fix invalid access to file *

[Jacopo Mondi]

Sice commit 7b9eb53e8591 ("media: cx18: Access v4l2_fh from file")
all ioctl handlers have been ported to operate on the file * first
function argument.

The cx18 DVB layer calls cx18_init_on_first_open() when the driver needs
to start streaming. This function calls the s_input(), s_std() and
s_frequency() ioctl handlers directly, but being called from the driver
context, it doesn't have a valid file * to pass them. This causes
the ioctl handlers to deference an invalid pointer.

Fix this by moving the implementation of those ioctls to functions that
take a cx18 pointer instead of a file pointer, and turn the V4L2 ioctl
handlers into wrappers that get the cx18 from the file. When calling
from cx18_init_on_first_open(), pass the cx18 pointer directly. This
allows removing the fake fh in cx18_init_on_first_open().

The bug has been reported by Smatch:

--> 1223         cx18_s_input(NULL, &fh, video_input);
The patch adds a new dereference of "file" but some of the callers pass a
NULL pointer.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aKL4OMWsESUdX8KQ@stanley.mountain/
Fixes: 7b9eb53e8591 ("media: cx18: Access v4l2_fh from file")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/stable/20250818-cx18-v4l2-fh-v1-1-6fe153760bce%40ideasonboard.com
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 drivers/media/pci/cx18/cx18-driver.c |  9 +++------
 drivers/media/pci/cx18/cx18-ioctl.c  | 30 +++++++++++++++++++-----------
 drivers/media/pci/cx18/cx18-ioctl.h  |  8 +++++---
 3 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/media/pci/cx18/cx18-driver.c b/drivers/media/pci/cx18/cx18-driver.c
index 743fcc9613744bfc1edeffc51e908fe88520405a..cd84dfcefcf971a7adb9aac2bafb9089dbe0f33f 100644
--- a/drivers/media/pci/cx18/cx18-driver.c
+++ b/drivers/media/pci/cx18/cx18-driver.c
@@ -1136,11 +1136,8 @@ int cx18_init_on_first_open(struct cx18 *cx)
 	int video_input;
 	int fw_retry_count = 3;
 	struct v4l2_frequency vf;
-	struct cx18_open_id fh;
 	v4l2_std_id std;
 
-	fh.cx = cx;
-
 	if (test_bit(CX18_F_I_FAILED, &cx->i_flags))
 		return -ENXIO;
 
@@ -1220,14 +1217,14 @@ int cx18_init_on_first_open(struct cx18 *cx)
 
 	video_input = cx->active_input;
 	cx->active_input++;	/* Force update of input */
-	cx18_s_input(NULL, &fh, video_input);
+	cx18_do_s_input(cx, video_input);
 
 	/* Let the VIDIOC_S_STD ioctl do all the work, keeps the code
 	   in one place. */
 	cx->std++;		/* Force full standard initialization */
 	std = (cx->tuner_std == V4L2_STD_ALL) ? V4L2_STD_NTSC_M : cx->tuner_std;
-	cx18_s_std(NULL, &fh, std);
-	cx18_s_frequency(NULL, &fh, &vf);
+	cx18_do_s_std(cx, std);
+	cx18_do_s_frequency(cx, &vf);
 	return 0;
 }
 
diff --git a/drivers/media/pci/cx18/cx18-ioctl.c b/drivers/media/pci/cx18/cx18-ioctl.c
index bf16d36448f888d9326b5f4a8f9c8f0e13d0c3a1..6e869c43cbd520feb720a71d8eb2dd60c05b0ae9 100644
--- a/drivers/media/pci/cx18/cx18-ioctl.c
+++ b/drivers/media/pci/cx18/cx18-ioctl.c
@@ -521,10 +521,8 @@ static int cx18_g_input(struct file *file, void *fh, unsigned int *i)
 	return 0;
 }
 
-int cx18_s_input(struct file *file, void *fh, unsigned int inp)
+int cx18_do_s_input(struct cx18 *cx, unsigned int inp)
 {
-	struct cx18_open_id *id = file2id(file);
-	struct cx18 *cx = id->cx;
 	v4l2_std_id std = V4L2_STD_ALL;
 	const struct cx18_card_video_input *card_input =
 				cx->card->video_inputs + inp;
@@ -558,6 +556,11 @@ int cx18_s_input(struct file *file, void *fh, unsigned int inp)
 	return 0;
 }
 
+static int cx18_s_input(struct file *file, void *fh, unsigned int inp)
+{
+	return cx18_do_s_input(file2id(file)->cx, inp);
+}
+
 static int cx18_g_frequency(struct file *file, void *fh,
 				struct v4l2_frequency *vf)
 {
@@ -570,11 +573,8 @@ static int cx18_g_frequency(struct file *file, void *fh,
 	return 0;
 }
 
-int cx18_s_frequency(struct file *file, void *fh, const struct v4l2_frequency *vf)
+int cx18_do_s_frequency(struct cx18 *cx, const struct v4l2_frequency *vf)
 {
-	struct cx18_open_id *id = file2id(file);
-	struct cx18 *cx = id->cx;
-
 	if (vf->tuner != 0)
 		return -EINVAL;
 
@@ -585,6 +585,12 @@ int cx18_s_frequency(struct file *file, void *fh, const struct v4l2_frequency *v
 	return 0;
 }
 
+static int cx18_s_frequency(struct file *file, void *fh,
+			    const struct v4l2_frequency *vf)
+{
+	return cx18_do_s_frequency(file2id(file)->cx, vf);
+}
+
 static int cx18_g_std(struct file *file, void *fh, v4l2_std_id *std)
 {
 	struct cx18 *cx = file2id(file)->cx;
@@ -593,11 +599,8 @@ static int cx18_g_std(struct file *file, void *fh, v4l2_std_id *std)
 	return 0;
 }
 
-int cx18_s_std(struct file *file, void *fh, v4l2_std_id std)
+int cx18_do_s_std(struct cx18 *cx, v4l2_std_id std)
 {
-	struct cx18_open_id *id = file2id(file);
-	struct cx18 *cx = id->cx;
-
 	if ((std & V4L2_STD_ALL) == 0)
 		return -EINVAL;
 
@@ -642,6 +645,11 @@ int cx18_s_std(struct file *file, void *fh, v4l2_std_id std)
 	return 0;
 }
 
+static int cx18_s_std(struct file *file, void *fh, v4l2_std_id std)
+{
+	return cx18_do_s_std(file2id(file)->cx, std);
+}
+
 static int cx18_s_tuner(struct file *file, void *fh, const struct v4l2_tuner *vt)
 {
 	struct cx18_open_id *id = file2id(file);
diff --git a/drivers/media/pci/cx18/cx18-ioctl.h b/drivers/media/pci/cx18/cx18-ioctl.h
index 221e2400fb3e2d817eaff7515fa89eb94f2d7f8a..7a42ac99312ab6502e1abe4f3d5c88c9c7f144f3 100644
--- a/drivers/media/pci/cx18/cx18-ioctl.h
+++ b/drivers/media/pci/cx18/cx18-ioctl.h
@@ -12,6 +12,8 @@ u16 cx18_service2vbi(int type);
 void cx18_expand_service_set(struct v4l2_sliced_vbi_format *fmt, int is_pal);
 u16 cx18_get_service_set(struct v4l2_sliced_vbi_format *fmt);
 void cx18_set_funcs(struct video_device *vdev);
-int cx18_s_std(struct file *file, void *fh, v4l2_std_id std);
-int cx18_s_frequency(struct file *file, void *fh, const struct v4l2_frequency *vf);
-int cx18_s_input(struct file *file, void *fh, unsigned int inp);
+
+struct cx18;
+int cx18_do_s_std(struct cx18 *cx, v4l2_std_id std);
+int cx18_do_s_frequency(struct cx18 *cx, const struct v4l2_frequency *vf);
+int cx18_do_s_input(struct cx18 *cx, unsigned int inp);

-- 
2.50.1

[PATCH v4 2/2] media: ivtv: Fix invalid access to file *

[Jacopo Mondi]

Since commit 9ba9d11544f9 ("media: ivtv: Access v4l2_fh from file")
all ioctl handlers have been ported to operate on the file * first
function argument.

The ivtv DVB layer calls ivtv_init_on_first_open() when the driver
needs to start streaming. This function calls the s_input() and
s_frequency() ioctl handlers directly, but being called from the driver
context, it doesn't have a valid file * to pass them. This causes the
ioctl handlers to deference an invalid pointer.

Fix this by moving the implementation of those ioctls to two helper
functions.

The ivtv_do_s_input() helper accepts a struct ivtv * as first argument,
which is easily accessible in ivtv_init_on_first_open() as well as from
the file * argument of the ioctl handler.

The ivtv_s_frequency() takes an ivtv_stream * instead. The stream * can
safely be accessed in ivtv_init_on_first_open() where it is hard-coded
to the IVTV_ENC_STREAM_TYPE_MPG stream type, as well as from the ioctl
handler as a valid stream type is associated to each open file handle
depending on which video device node has been opened in the ivtv_open()
file operation.

The bug has been reported by Smatch.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aKL4OMWsESUdX8KQ@stanley.mountain/
Fixes: 9ba9d11544f9 ("media: ivtv: Access v4l2_fh from file")
Cc: stable@vger.kernel.org
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 drivers/media/pci/ivtv/ivtv-driver.c | 11 ++++-------
 drivers/media/pci/ivtv/ivtv-ioctl.c  | 22 +++++++++++++++++-----
 drivers/media/pci/ivtv/ivtv-ioctl.h  |  6 ++++--
 3 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-driver.c b/drivers/media/pci/ivtv/ivtv-driver.c
index ecc20cd89926fe2ce4e472526a6b5fc0857615dd..7e2fb98cfccf02f701ceb4484dd1d330dd1dc867 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.c
+++ b/drivers/media/pci/ivtv/ivtv-driver.c
@@ -1260,15 +1260,12 @@ static int ivtv_probe(struct pci_dev *pdev, const struct pci_device_id *pci_id)
 
 int ivtv_init_on_first_open(struct ivtv *itv)
 {
-	struct v4l2_frequency vf;
 	/* Needed to call ioctls later */
-	struct ivtv_open_id fh;
+	struct ivtv_stream *s = &itv->streams[IVTV_ENC_STREAM_TYPE_MPG];
+	struct v4l2_frequency vf;
 	int fw_retry_count = 3;
 	int video_input;
 
-	fh.itv = itv;
-	fh.type = IVTV_ENC_STREAM_TYPE_MPG;
-
 	if (test_bit(IVTV_F_I_FAILED, &itv->i_flags))
 		return -ENXIO;
 
@@ -1310,13 +1307,13 @@ int ivtv_init_on_first_open(struct ivtv *itv)
 
 	video_input = itv->active_input;
 	itv->active_input++;	/* Force update of input */
-	ivtv_s_input(NULL, &fh, video_input);
+	ivtv_do_s_input(itv, video_input);
 
 	/* Let the VIDIOC_S_STD ioctl do all the work, keeps the code
 	   in one place. */
 	itv->std++;		/* Force full standard initialization */
 	itv->std_out = itv->std;
-	ivtv_s_frequency(NULL, &fh, &vf);
+	ivtv_do_s_frequency(s, &vf);
 
 	if (itv->card->v4l2_capabilities & V4L2_CAP_VIDEO_OUTPUT) {
 		/* Turn on the TV-out: ivtv_init_mpeg_decoder() initializes
diff --git a/drivers/media/pci/ivtv/ivtv-ioctl.c b/drivers/media/pci/ivtv/ivtv-ioctl.c
index 8077a71d4850ec773caa20c3fca08f92f3117d69..dfbc842b22453868a2075935a81db7ae313ee46c 100644
--- a/drivers/media/pci/ivtv/ivtv-ioctl.c
+++ b/drivers/media/pci/ivtv/ivtv-ioctl.c
@@ -974,9 +974,8 @@ static int ivtv_g_input(struct file *file, void *fh, unsigned int *i)
 	return 0;
 }
 
-int ivtv_s_input(struct file *file, void *fh, unsigned int inp)
+int ivtv_do_s_input(struct ivtv *itv, unsigned int inp)
 {
-	struct ivtv *itv = file2id(file)->itv;
 	v4l2_std_id std;
 	int i;
 
@@ -1017,6 +1016,11 @@ int ivtv_s_input(struct file *file, void *fh, unsigned int inp)
 	return 0;
 }
 
+static int ivtv_s_input(struct file *file, void *fh, unsigned int inp)
+{
+	return ivtv_do_s_input(file2id(file)->itv, inp);
+}
+
 static int ivtv_g_output(struct file *file, void *fh, unsigned int *i)
 {
 	struct ivtv *itv = file2id(file)->itv;
@@ -1065,10 +1069,9 @@ static int ivtv_g_frequency(struct file *file, void *fh, struct v4l2_frequency *
 	return 0;
 }
 
-int ivtv_s_frequency(struct file *file, void *fh, const struct v4l2_frequency *vf)
+int ivtv_do_s_frequency(struct ivtv_stream *s, const struct v4l2_frequency *vf)
 {
-	struct ivtv *itv = file2id(file)->itv;
-	struct ivtv_stream *s = &itv->streams[file2id(file)->type];
+	struct ivtv *itv = s->itv;
 
 	if (s->vdev.vfl_dir)
 		return -ENOTTY;
@@ -1082,6 +1085,15 @@ int ivtv_s_frequency(struct file *file, void *fh, const struct v4l2_frequency *v
 	return 0;
 }
 
+static int ivtv_s_frequency(struct file *file, void *fh,
+			    const struct v4l2_frequency *vf)
+{
+	struct ivtv_open_id *id = file2id(file);
+	struct ivtv *itv = id->itv;
+
+	return ivtv_do_s_frequency(&itv->streams[id->type], vf);
+}
+
 static int ivtv_g_std(struct file *file, void *fh, v4l2_std_id *std)
 {
 	struct ivtv *itv = file2id(file)->itv;
diff --git a/drivers/media/pci/ivtv/ivtv-ioctl.h b/drivers/media/pci/ivtv/ivtv-ioctl.h
index 42c2516379fcbbd0640820ab0e3abe9bf00b57ea..edc05eb8e060fd64d7ff94f8f7f5c315a2fa6298 100644
--- a/drivers/media/pci/ivtv/ivtv-ioctl.h
+++ b/drivers/media/pci/ivtv/ivtv-ioctl.h
@@ -9,6 +9,8 @@
 #ifndef IVTV_IOCTL_H
 #define IVTV_IOCTL_H
 
+struct ivtv;
+
 u16 ivtv_service2vbi(int type);
 void ivtv_expand_service_set(struct v4l2_sliced_vbi_format *fmt, int is_pal);
 u16 ivtv_get_service_set(struct v4l2_sliced_vbi_format *fmt);
@@ -17,7 +19,7 @@ int ivtv_set_speed(struct ivtv *itv, int speed);
 void ivtv_set_funcs(struct video_device *vdev);
 void ivtv_s_std_enc(struct ivtv *itv, v4l2_std_id std);
 void ivtv_s_std_dec(struct ivtv *itv, v4l2_std_id std);
-int ivtv_s_frequency(struct file *file, void *fh, const struct v4l2_frequency *vf);
-int ivtv_s_input(struct file *file, void *fh, unsigned int inp);
+int ivtv_do_s_frequency(struct ivtv_stream *s, const struct v4l2_frequency *vf);
+int ivtv_do_s_input(struct ivtv *itv, unsigned int inp);
 
 #endif

-- 
2.50.1

Re: [PATCH v4 0/2] media: pci: Fix invalid access to file *

[Hans Verkuil]

Hi Jacopo,

On 19/08/2025 09:07, Jacopo Mondi wrote:
> Since commits
> 7b9eb53e8591 ("media: cx18: Access v4l2_fh from file")
> 9ba9d11544f9 ("media: ivtv: Access v4l2_fh from file")
> 
> All the ioctl handlers access their private data structures
> from file *
> 
> The ivtv and cx18 drivers call the ioctl handlers from their
> DVB layer without a valid file *, causing invalid memory access.
> 
> The issue has been reported by smatch in
> "[bug report] media: cx18: Access v4l2_fh from file"
> 
> Fix this by providing wrappers for the ioctl handlers to be
> used by the DVB layer that do not require a valid file *.

This series should go to the fixes branch for v6.18, right?
This looks like a pure regression, so I think that makes sense.

BTW, why is there a Link: tag in the cx18 patch? It just links to
the v1 of the patch and that doesn't add meaningful information.
Linus likes Link:, but only if it really adds useful information.

Regards,

	Hans

> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
> Changes in v4:
> - Slightly adjust commit messages
> - Link to v3: https://lore.kernel.org/r/20250818-cx18-v4l2-fh-v3-0-5e2f08f3cadc@ideasonboard.com
> 
> Changes in v3:
> - Change helpers to accept the type they're going to operate on instead
>   of using the open_id wrapper type as suggested by Laurent
> - Link to v2: https://lore.kernel.org/r/20250818-cx18-v4l2-fh-v2-0-3f53ce423663@ideasonboard.com
> 
> Changes in v2:
> - Add Cc: stable@vger.kernel.org per-patch
> 
> ---
> Jacopo Mondi (2):
>       media: cx18: Fix invalid access to file *
>       media: ivtv: Fix invalid access to file *
> 
>  drivers/media/pci/cx18/cx18-driver.c |  9 +++------
>  drivers/media/pci/cx18/cx18-ioctl.c  | 30 +++++++++++++++++++-----------
>  drivers/media/pci/cx18/cx18-ioctl.h  |  8 +++++---
>  drivers/media/pci/ivtv/ivtv-driver.c | 11 ++++-------
>  drivers/media/pci/ivtv/ivtv-ioctl.c  | 22 +++++++++++++++++-----
>  drivers/media/pci/ivtv/ivtv-ioctl.h  |  6 ++++--
>  6 files changed, 52 insertions(+), 34 deletions(-)
> ---
> base-commit: a75b8d198c55e9eb5feb6f6e155496305caba2dc
> change-id: 20250818-cx18-v4l2-fh-7eaa6199fdde
> 
> Best regards,

Re: [PATCH v4 0/2] media: pci: Fix invalid access to file *

[Jacopo Mondi]

Hi Hans

On Tue, Oct 14, 2025 at 09:05:20AM +0200, Hans Verkuil wrote:
> Hi Jacopo,
>
> On 19/08/2025 09:07, Jacopo Mondi wrote:
> > Since commits
> > 7b9eb53e8591 ("media: cx18: Access v4l2_fh from file")
> > 9ba9d11544f9 ("media: ivtv: Access v4l2_fh from file")
> >
> > All the ioctl handlers access their private data structures
> > from file *
> >
> > The ivtv and cx18 drivers call the ioctl handlers from their
> > DVB layer without a valid file *, causing invalid memory access.
> >
> > The issue has been reported by smatch in
> > "[bug report] media: cx18: Access v4l2_fh from file"
> >
> > Fix this by providing wrappers for the ioctl handlers to be
> > used by the DVB layer that do not require a valid file *.
>
> This series should go to the fixes branch for v6.18, right?
> This looks like a pure regression, so I think that makes sense.
>

I think so, yes

> BTW, why is there a Link: tag in the cx18 patch? It just links to
> the v1 of the patch and that doesn't add meaningful information.
> Linus likes Link:, but only if it really adds useful information.

Good question. I presume it's probably a copy&paste error, as it has no
place in the patch.

Would you like me to resend or will you remove it ?


>
> Regards,
>
> 	Hans
>
> >
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > ---
> > Changes in v4:
> > - Slightly adjust commit messages
> > - Link to v3: https://lore.kernel.org/r/20250818-cx18-v4l2-fh-v3-0-5e2f08f3cadc@ideasonboard.com
> >
> > Changes in v3:
> > - Change helpers to accept the type they're going to operate on instead
> >   of using the open_id wrapper type as suggested by Laurent
> > - Link to v2: https://lore.kernel.org/r/20250818-cx18-v4l2-fh-v2-0-3f53ce423663@ideasonboard.com
> >
> > Changes in v2:
> > - Add Cc: stable@vger.kernel.org per-patch
> >
> > ---
> > Jacopo Mondi (2):
> >       media: cx18: Fix invalid access to file *
> >       media: ivtv: Fix invalid access to file *
> >
> >  drivers/media/pci/cx18/cx18-driver.c |  9 +++------
> >  drivers/media/pci/cx18/cx18-ioctl.c  | 30 +++++++++++++++++++-----------
> >  drivers/media/pci/cx18/cx18-ioctl.h  |  8 +++++---
> >  drivers/media/pci/ivtv/ivtv-driver.c | 11 ++++-------
> >  drivers/media/pci/ivtv/ivtv-ioctl.c  | 22 +++++++++++++++++-----
> >  drivers/media/pci/ivtv/ivtv-ioctl.h  |  6 ++++--
> >  6 files changed, 52 insertions(+), 34 deletions(-)
> > ---
> > base-commit: a75b8d198c55e9eb5feb6f6e155496305caba2dc
> > change-id: 20250818-cx18-v4l2-fh-7eaa6199fdde
> >
> > Best regards,
>

Re: [PATCH v4 0/2] media: pci: Fix invalid access to file *

[Hans Verkuil]

On 14/10/2025 13:52, Jacopo Mondi wrote:
> Hi Hans
> 
> On Tue, Oct 14, 2025 at 09:05:20AM +0200, Hans Verkuil wrote:
>> Hi Jacopo,
>>
>> On 19/08/2025 09:07, Jacopo Mondi wrote:
>>> Since commits
>>> 7b9eb53e8591 ("media: cx18: Access v4l2_fh from file")
>>> 9ba9d11544f9 ("media: ivtv: Access v4l2_fh from file")
>>>
>>> All the ioctl handlers access their private data structures
>>> from file *
>>>
>>> The ivtv and cx18 drivers call the ioctl handlers from their
>>> DVB layer without a valid file *, causing invalid memory access.
>>>
>>> The issue has been reported by smatch in
>>> "[bug report] media: cx18: Access v4l2_fh from file"
>>>
>>> Fix this by providing wrappers for the ioctl handlers to be
>>> used by the DVB layer that do not require a valid file *.
>>
>> This series should go to the fixes branch for v6.18, right?
>> This looks like a pure regression, so I think that makes sense.
>>
> 
> I think so, yes
> 
>> BTW, why is there a Link: tag in the cx18 patch? It just links to
>> the v1 of the patch and that doesn't add meaningful information.
>> Linus likes Link:, but only if it really adds useful information.
> 
> Good question. I presume it's probably a copy&paste error, as it has no
> place in the patch.
> 
> Would you like me to resend or will you remove it ?

I would prefer a resend for these two patches, clearly marked as [PATCH for v6.18 v5 n/2]
in the subject.

Normally I'd just drop the Link: tag, but it's good to have this clearly marked
as a fix for v6.18.

I'll also see if I can do a quick test of these two patches with my ivtv and cx18
boards. Just to make sure there are no other corner cases lurking in these drivers.

Regards,

	Hans

> 
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>>> ---
>>> Changes in v4:
>>> - Slightly adjust commit messages
>>> - Link to v3: https://lore.kernel.org/r/20250818-cx18-v4l2-fh-v3-0-5e2f08f3cadc@ideasonboard.com
>>>
>>> Changes in v3:
>>> - Change helpers to accept the type they're going to operate on instead
>>>   of using the open_id wrapper type as suggested by Laurent
>>> - Link to v2: https://lore.kernel.org/r/20250818-cx18-v4l2-fh-v2-0-3f53ce423663@ideasonboard.com
>>>
>>> Changes in v2:
>>> - Add Cc: stable@vger.kernel.org per-patch
>>>
>>> ---
>>> Jacopo Mondi (2):
>>>       media: cx18: Fix invalid access to file *
>>>       media: ivtv: Fix invalid access to file *
>>>
>>>  drivers/media/pci/cx18/cx18-driver.c |  9 +++------
>>>  drivers/media/pci/cx18/cx18-ioctl.c  | 30 +++++++++++++++++++-----------
>>>  drivers/media/pci/cx18/cx18-ioctl.h  |  8 +++++---
>>>  drivers/media/pci/ivtv/ivtv-driver.c | 11 ++++-------
>>>  drivers/media/pci/ivtv/ivtv-ioctl.c  | 22 +++++++++++++++++-----
>>>  drivers/media/pci/ivtv/ivtv-ioctl.h  |  6 ++++--
>>>  6 files changed, 52 insertions(+), 34 deletions(-)
>>> ---
>>> base-commit: a75b8d198c55e9eb5feb6f6e155496305caba2dc
>>> change-id: 20250818-cx18-v4l2-fh-7eaa6199fdde
>>>
>>> Best regards,
>>

Re: [PATCH v4 2/2] media: ivtv: Fix invalid access to file *

[Hans Verkuil]

On 19/08/2025 09:07, Jacopo Mondi wrote:
> Since commit 9ba9d11544f9 ("media: ivtv: Access v4l2_fh from file")
> all ioctl handlers have been ported to operate on the file * first
> function argument.
> 
> The ivtv DVB layer calls ivtv_init_on_first_open() when the driver
> needs to start streaming. This function calls the s_input() and
> s_frequency() ioctl handlers directly, but being called from the driver
> context, it doesn't have a valid file * to pass them. This causes the
> ioctl handlers to deference an invalid pointer.
> 
> Fix this by moving the implementation of those ioctls to two helper
> functions.
> 
> The ivtv_do_s_input() helper accepts a struct ivtv * as first argument,
> which is easily accessible in ivtv_init_on_first_open() as well as from
> the file * argument of the ioctl handler.
> 
> The ivtv_s_frequency() takes an ivtv_stream * instead. The stream * can
> safely be accessed in ivtv_init_on_first_open() where it is hard-coded
> to the IVTV_ENC_STREAM_TYPE_MPG stream type, as well as from the ioctl
> handler as a valid stream type is associated to each open file handle
> depending on which video device node has been opened in the ivtv_open()
> file operation.
> 
> The bug has been reported by Smatch.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/aKL4OMWsESUdX8KQ@stanley.mountain/
> Fixes: 9ba9d11544f9 ("media: ivtv: Access v4l2_fh from file")
> Cc: stable@vger.kernel.org
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Tested-by: Hans Verkuil <hverkuil+cisco@kernel.org>

I'll try the cx18 board next :-)

Regards,

	Hans

> ---
>  drivers/media/pci/ivtv/ivtv-driver.c | 11 ++++-------
>  drivers/media/pci/ivtv/ivtv-ioctl.c  | 22 +++++++++++++++++-----
>  drivers/media/pci/ivtv/ivtv-ioctl.h  |  6 ++++--
>  3 files changed, 25 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/pci/ivtv/ivtv-driver.c b/drivers/media/pci/ivtv/ivtv-driver.c
> index ecc20cd89926fe2ce4e472526a6b5fc0857615dd..7e2fb98cfccf02f701ceb4484dd1d330dd1dc867 100644
> --- a/drivers/media/pci/ivtv/ivtv-driver.c
> +++ b/drivers/media/pci/ivtv/ivtv-driver.c
> @@ -1260,15 +1260,12 @@ static int ivtv_probe(struct pci_dev *pdev, const struct pci_device_id *pci_id)
>  
>  int ivtv_init_on_first_open(struct ivtv *itv)
>  {
> -	struct v4l2_frequency vf;
>  	/* Needed to call ioctls later */
> -	struct ivtv_open_id fh;
> +	struct ivtv_stream *s = &itv->streams[IVTV_ENC_STREAM_TYPE_MPG];
> +	struct v4l2_frequency vf;
>  	int fw_retry_count = 3;
>  	int video_input;
>  
> -	fh.itv = itv;
> -	fh.type = IVTV_ENC_STREAM_TYPE_MPG;
> -
>  	if (test_bit(IVTV_F_I_FAILED, &itv->i_flags))
>  		return -ENXIO;
>  
> @@ -1310,13 +1307,13 @@ int ivtv_init_on_first_open(struct ivtv *itv)
>  
>  	video_input = itv->active_input;
>  	itv->active_input++;	/* Force update of input */
> -	ivtv_s_input(NULL, &fh, video_input);
> +	ivtv_do_s_input(itv, video_input);
>  
>  	/* Let the VIDIOC_S_STD ioctl do all the work, keeps the code
>  	   in one place. */
>  	itv->std++;		/* Force full standard initialization */
>  	itv->std_out = itv->std;
> -	ivtv_s_frequency(NULL, &fh, &vf);
> +	ivtv_do_s_frequency(s, &vf);
>  
>  	if (itv->card->v4l2_capabilities & V4L2_CAP_VIDEO_OUTPUT) {
>  		/* Turn on the TV-out: ivtv_init_mpeg_decoder() initializes
> diff --git a/drivers/media/pci/ivtv/ivtv-ioctl.c b/drivers/media/pci/ivtv/ivtv-ioctl.c
> index 8077a71d4850ec773caa20c3fca08f92f3117d69..dfbc842b22453868a2075935a81db7ae313ee46c 100644
> --- a/drivers/media/pci/ivtv/ivtv-ioctl.c
> +++ b/drivers/media/pci/ivtv/ivtv-ioctl.c
> @@ -974,9 +974,8 @@ static int ivtv_g_input(struct file *file, void *fh, unsigned int *i)
>  	return 0;
>  }
>  
> -int ivtv_s_input(struct file *file, void *fh, unsigned int inp)
> +int ivtv_do_s_input(struct ivtv *itv, unsigned int inp)
>  {
> -	struct ivtv *itv = file2id(file)->itv;
>  	v4l2_std_id std;
>  	int i;
>  
> @@ -1017,6 +1016,11 @@ int ivtv_s_input(struct file *file, void *fh, unsigned int inp)
>  	return 0;
>  }
>  
> +static int ivtv_s_input(struct file *file, void *fh, unsigned int inp)
> +{
> +	return ivtv_do_s_input(file2id(file)->itv, inp);
> +}
> +
>  static int ivtv_g_output(struct file *file, void *fh, unsigned int *i)
>  {
>  	struct ivtv *itv = file2id(file)->itv;
> @@ -1065,10 +1069,9 @@ static int ivtv_g_frequency(struct file *file, void *fh, struct v4l2_frequency *
>  	return 0;
>  }
>  
> -int ivtv_s_frequency(struct file *file, void *fh, const struct v4l2_frequency *vf)
> +int ivtv_do_s_frequency(struct ivtv_stream *s, const struct v4l2_frequency *vf)
>  {
> -	struct ivtv *itv = file2id(file)->itv;
> -	struct ivtv_stream *s = &itv->streams[file2id(file)->type];
> +	struct ivtv *itv = s->itv;
>  
>  	if (s->vdev.vfl_dir)
>  		return -ENOTTY;
> @@ -1082,6 +1085,15 @@ int ivtv_s_frequency(struct file *file, void *fh, const struct v4l2_frequency *v
>  	return 0;
>  }
>  
> +static int ivtv_s_frequency(struct file *file, void *fh,
> +			    const struct v4l2_frequency *vf)
> +{
> +	struct ivtv_open_id *id = file2id(file);
> +	struct ivtv *itv = id->itv;
> +
> +	return ivtv_do_s_frequency(&itv->streams[id->type], vf);
> +}
> +
>  static int ivtv_g_std(struct file *file, void *fh, v4l2_std_id *std)
>  {
>  	struct ivtv *itv = file2id(file)->itv;
> diff --git a/drivers/media/pci/ivtv/ivtv-ioctl.h b/drivers/media/pci/ivtv/ivtv-ioctl.h
> index 42c2516379fcbbd0640820ab0e3abe9bf00b57ea..edc05eb8e060fd64d7ff94f8f7f5c315a2fa6298 100644
> --- a/drivers/media/pci/ivtv/ivtv-ioctl.h
> +++ b/drivers/media/pci/ivtv/ivtv-ioctl.h
> @@ -9,6 +9,8 @@
>  #ifndef IVTV_IOCTL_H
>  #define IVTV_IOCTL_H
>  
> +struct ivtv;
> +
>  u16 ivtv_service2vbi(int type);
>  void ivtv_expand_service_set(struct v4l2_sliced_vbi_format *fmt, int is_pal);
>  u16 ivtv_get_service_set(struct v4l2_sliced_vbi_format *fmt);
> @@ -17,7 +19,7 @@ int ivtv_set_speed(struct ivtv *itv, int speed);
>  void ivtv_set_funcs(struct video_device *vdev);
>  void ivtv_s_std_enc(struct ivtv *itv, v4l2_std_id std);
>  void ivtv_s_std_dec(struct ivtv *itv, v4l2_std_id std);
> -int ivtv_s_frequency(struct file *file, void *fh, const struct v4l2_frequency *vf);
> -int ivtv_s_input(struct file *file, void *fh, unsigned int inp);
> +int ivtv_do_s_frequency(struct ivtv_stream *s, const struct v4l2_frequency *vf);
> +int ivtv_do_s_input(struct ivtv *itv, unsigned int inp);
>  
>  #endif
> 

Re: [PATCH v4 1/2] media: cx18: Fix invalid access to file *

[Hans Verkuil]

On 19/08/2025 09:07, Jacopo Mondi wrote:
> Sice commit 7b9eb53e8591 ("media: cx18: Access v4l2_fh from file")
> all ioctl handlers have been ported to operate on the file * first
> function argument.
> 
> The cx18 DVB layer calls cx18_init_on_first_open() when the driver needs
> to start streaming. This function calls the s_input(), s_std() and
> s_frequency() ioctl handlers directly, but being called from the driver
> context, it doesn't have a valid file * to pass them. This causes
> the ioctl handlers to deference an invalid pointer.
> 
> Fix this by moving the implementation of those ioctls to functions that
> take a cx18 pointer instead of a file pointer, and turn the V4L2 ioctl
> handlers into wrappers that get the cx18 from the file. When calling
> from cx18_init_on_first_open(), pass the cx18 pointer directly. This
> allows removing the fake fh in cx18_init_on_first_open().
> 
> The bug has been reported by Smatch:
> 
> --> 1223         cx18_s_input(NULL, &fh, video_input);
> The patch adds a new dereference of "file" but some of the callers pass a
> NULL pointer.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/aKL4OMWsESUdX8KQ@stanley.mountain/
> Fixes: 7b9eb53e8591 ("media: cx18: Access v4l2_fh from file")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/stable/20250818-cx18-v4l2-fh-v1-1-6fe153760bce%40ideasonboard.com
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Tested-by: Hans Verkuil <hverkuil+cisco@kernel.org>

Regards,

	Hans

> ---
>  drivers/media/pci/cx18/cx18-driver.c |  9 +++------
>  drivers/media/pci/cx18/cx18-ioctl.c  | 30 +++++++++++++++++++-----------
>  drivers/media/pci/cx18/cx18-ioctl.h  |  8 +++++---
>  3 files changed, 27 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/pci/cx18/cx18-driver.c b/drivers/media/pci/cx18/cx18-driver.c
> index 743fcc9613744bfc1edeffc51e908fe88520405a..cd84dfcefcf971a7adb9aac2bafb9089dbe0f33f 100644
> --- a/drivers/media/pci/cx18/cx18-driver.c
> +++ b/drivers/media/pci/cx18/cx18-driver.c
> @@ -1136,11 +1136,8 @@ int cx18_init_on_first_open(struct cx18 *cx)
>  	int video_input;
>  	int fw_retry_count = 3;
>  	struct v4l2_frequency vf;
> -	struct cx18_open_id fh;
>  	v4l2_std_id std;
>  
> -	fh.cx = cx;
> -
>  	if (test_bit(CX18_F_I_FAILED, &cx->i_flags))
>  		return -ENXIO;
>  
> @@ -1220,14 +1217,14 @@ int cx18_init_on_first_open(struct cx18 *cx)
>  
>  	video_input = cx->active_input;
>  	cx->active_input++;	/* Force update of input */
> -	cx18_s_input(NULL, &fh, video_input);
> +	cx18_do_s_input(cx, video_input);
>  
>  	/* Let the VIDIOC_S_STD ioctl do all the work, keeps the code
>  	   in one place. */
>  	cx->std++;		/* Force full standard initialization */
>  	std = (cx->tuner_std == V4L2_STD_ALL) ? V4L2_STD_NTSC_M : cx->tuner_std;
> -	cx18_s_std(NULL, &fh, std);
> -	cx18_s_frequency(NULL, &fh, &vf);
> +	cx18_do_s_std(cx, std);
> +	cx18_do_s_frequency(cx, &vf);
>  	return 0;
>  }
>  
> diff --git a/drivers/media/pci/cx18/cx18-ioctl.c b/drivers/media/pci/cx18/cx18-ioctl.c
> index bf16d36448f888d9326b5f4a8f9c8f0e13d0c3a1..6e869c43cbd520feb720a71d8eb2dd60c05b0ae9 100644
> --- a/drivers/media/pci/cx18/cx18-ioctl.c
> +++ b/drivers/media/pci/cx18/cx18-ioctl.c
> @@ -521,10 +521,8 @@ static int cx18_g_input(struct file *file, void *fh, unsigned int *i)
>  	return 0;
>  }
>  
> -int cx18_s_input(struct file *file, void *fh, unsigned int inp)
> +int cx18_do_s_input(struct cx18 *cx, unsigned int inp)
>  {
> -	struct cx18_open_id *id = file2id(file);
> -	struct cx18 *cx = id->cx;
>  	v4l2_std_id std = V4L2_STD_ALL;
>  	const struct cx18_card_video_input *card_input =
>  				cx->card->video_inputs + inp;
> @@ -558,6 +556,11 @@ int cx18_s_input(struct file *file, void *fh, unsigned int inp)
>  	return 0;
>  }
>  
> +static int cx18_s_input(struct file *file, void *fh, unsigned int inp)
> +{
> +	return cx18_do_s_input(file2id(file)->cx, inp);
> +}
> +
>  static int cx18_g_frequency(struct file *file, void *fh,
>  				struct v4l2_frequency *vf)
>  {
> @@ -570,11 +573,8 @@ static int cx18_g_frequency(struct file *file, void *fh,
>  	return 0;
>  }
>  
> -int cx18_s_frequency(struct file *file, void *fh, const struct v4l2_frequency *vf)
> +int cx18_do_s_frequency(struct cx18 *cx, const struct v4l2_frequency *vf)
>  {
> -	struct cx18_open_id *id = file2id(file);
> -	struct cx18 *cx = id->cx;
> -
>  	if (vf->tuner != 0)
>  		return -EINVAL;
>  
> @@ -585,6 +585,12 @@ int cx18_s_frequency(struct file *file, void *fh, const struct v4l2_frequency *v
>  	return 0;
>  }
>  
> +static int cx18_s_frequency(struct file *file, void *fh,
> +			    const struct v4l2_frequency *vf)
> +{
> +	return cx18_do_s_frequency(file2id(file)->cx, vf);
> +}
> +
>  static int cx18_g_std(struct file *file, void *fh, v4l2_std_id *std)
>  {
>  	struct cx18 *cx = file2id(file)->cx;
> @@ -593,11 +599,8 @@ static int cx18_g_std(struct file *file, void *fh, v4l2_std_id *std)
>  	return 0;
>  }
>  
> -int cx18_s_std(struct file *file, void *fh, v4l2_std_id std)
> +int cx18_do_s_std(struct cx18 *cx, v4l2_std_id std)
>  {
> -	struct cx18_open_id *id = file2id(file);
> -	struct cx18 *cx = id->cx;
> -
>  	if ((std & V4L2_STD_ALL) == 0)
>  		return -EINVAL;
>  
> @@ -642,6 +645,11 @@ int cx18_s_std(struct file *file, void *fh, v4l2_std_id std)
>  	return 0;
>  }
>  
> +static int cx18_s_std(struct file *file, void *fh, v4l2_std_id std)
> +{
> +	return cx18_do_s_std(file2id(file)->cx, std);
> +}
> +
>  static int cx18_s_tuner(struct file *file, void *fh, const struct v4l2_tuner *vt)
>  {
>  	struct cx18_open_id *id = file2id(file);
> diff --git a/drivers/media/pci/cx18/cx18-ioctl.h b/drivers/media/pci/cx18/cx18-ioctl.h
> index 221e2400fb3e2d817eaff7515fa89eb94f2d7f8a..7a42ac99312ab6502e1abe4f3d5c88c9c7f144f3 100644
> --- a/drivers/media/pci/cx18/cx18-ioctl.h
> +++ b/drivers/media/pci/cx18/cx18-ioctl.h
> @@ -12,6 +12,8 @@ u16 cx18_service2vbi(int type);
>  void cx18_expand_service_set(struct v4l2_sliced_vbi_format *fmt, int is_pal);
>  u16 cx18_get_service_set(struct v4l2_sliced_vbi_format *fmt);
>  void cx18_set_funcs(struct video_device *vdev);
> -int cx18_s_std(struct file *file, void *fh, v4l2_std_id std);
> -int cx18_s_frequency(struct file *file, void *fh, const struct v4l2_frequency *vf);
> -int cx18_s_input(struct file *file, void *fh, unsigned int inp);
> +
> +struct cx18;
> +int cx18_do_s_std(struct cx18 *cx, v4l2_std_id std);
> +int cx18_do_s_frequency(struct cx18 *cx, const struct v4l2_frequency *vf);
> +int cx18_do_s_input(struct cx18 *cx, unsigned int inp);
>