[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a03637f5-a810-4620-8ba3-d502a4ce7dd3@kernel.org>
Date: Tue, 14 Oct 2025 14:34:09 +0200
From: Hans Verkuil <hverkuil+cisco@...nel.org>
To: Jacopo Mondi <jacopo.mondi@...asonboard.com>,
Andy Walls <awalls@...metrocast.net>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>
Cc: Dan Carpenter <dan.carpenter@...aro.org>, stable@...r.kernel.org,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/2] media: ivtv: Fix invalid access to file *
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@...aro.org>
> Closes: https://lore.kernel.org/all/aKL4OMWsESUdX8KQ@stanley.mountain/
> Fixes: 9ba9d11544f9 ("media: ivtv: Access v4l2_fh from file")
> Cc: stable@...r.kernel.org
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@...asonboard.com>
Tested-by: Hans Verkuil <hverkuil+cisco@...nel.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
>
Powered by blists - more mailing lists