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);
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
linux-stable-mirror@lists.linaro.org