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@ideasonboa...
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@ideasonboa...
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,
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%40ide... 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);
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%40ide... 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);
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
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
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@ideasonboa...
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@ideasonboa...
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,
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@ideasonboa...
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@ideasonboa...
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,
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@ideasonboa...
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@ideasonboa...
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,
linux-stable-mirror@lists.linaro.org