linux-kernel - Re: [PATCH v2 2/2] media: ivtv: Fix invalid access to file *

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250818131514.GE5862@pendragon.ideasonboard.com>
Date: Mon, 18 Aug 2025 16:15:14 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Jacopo Mondi <jacopo.mondi@...asonboard.com>
Cc: Andy Walls <awalls@...metrocast.net>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Hans Verkuil <hverkuil+cisco@...nel.org>,
	Dan Carpenter <dan.carpenter@...aro.org>, stable@...r.kernel.org,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] media: ivtv: Fix invalid access to file *

On Mon, Aug 18, 2025 at 03:06:36PM +0200, 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 wrapping the ioctl handlers implementation in helper
> functions which accepts a ivtv_open_id pointer as first argument
> and make the ivtv_init_on_first_open() function call the helpers
> without going through the ioctl handlers.
> 
> 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
> Signed-off-by: Jacopo Mondi <jacopo.mondi@...asonboard.com>
> ---
>  drivers/media/pci/ivtv/ivtv-driver.c |  4 ++--
>  drivers/media/pci/ivtv/ivtv-ioctl.c  | 22 +++++++++++++++++-----
>  drivers/media/pci/ivtv/ivtv-ioctl.h  |  6 ++++--
>  3 files changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/pci/ivtv/ivtv-driver.c b/drivers/media/pci/ivtv/ivtv-driver.c
> index ecc20cd89926fe2ce4e472526a6b5fc0857615dd..bcbb03271047df9b127039759dbfefbaae67f9c3 100644
> --- a/drivers/media/pci/ivtv/ivtv-driver.c
> +++ b/drivers/media/pci/ivtv/ivtv-driver.c
> @@ -1310,13 +1310,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(&fh, 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(&fh, &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..ba1fce42a8d6d7d9a3779bfc9dfd310060e61103 100644
> --- a/drivers/media/pci/ivtv/ivtv-ioctl.c
> +++ b/drivers/media/pci/ivtv/ivtv-ioctl.c
> @@ -974,9 +974,9 @@ 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_open_id *id, unsigned int inp)
>  {
> -	struct ivtv *itv = file2id(file)->itv;
> +	struct ivtv *itv = id->itv;
>  	v4l2_std_id std;
>  	int i;
>  
> @@ -1017,6 +1017,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), inp);
> +}
> +
>  static int ivtv_g_output(struct file *file, void *fh, unsigned int *i)
>  {
>  	struct ivtv *itv = file2id(file)->itv;
> @@ -1065,10 +1070,11 @@ 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_open_id *id,
> +			const struct v4l2_frequency *vf)
>  {
> -	struct ivtv *itv = file2id(file)->itv;
> -	struct ivtv_stream *s = &itv->streams[file2id(file)->type];
> +	struct ivtv *itv = id->itv;
> +	struct ivtv_stream *s = &itv->streams[id->type];

Same comments as for 1/2. Additionally, you can pass the ivtv_stream
pointer to this function, either along with the ivtv pointer, or alone
as you can access ivtv through s->itv.

>  
>  	if (s->vdev.vfl_dir)
>  		return -ENOTTY;
> @@ -1082,6 +1088,12 @@ 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)
> +{
> +	return ivtv_do_s_frequency(file2id(file), 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..7c91b8963b420761c2afcab1dece4d19a4ec0322 100644
> --- a/drivers/media/pci/ivtv/ivtv-ioctl.h
> +++ b/drivers/media/pci/ivtv/ivtv-ioctl.h
> @@ -17,7 +17,9 @@ 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);
> +
> +struct ivtv_open_id;
> +int ivtv_do_s_frequency(struct ivtv_open_id *id, const struct v4l2_frequency *vf);
> +int ivtv_do_s_input(struct ivtv_open_id *id, unsigned int inp);
>  
>  #endif
> 

-- 
Regards,

Laurent Pinchart