| 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
| ||
|
Message-ID: <54a970ec-6c06-4544-babb-7ad30a011ad9@kernel.org>
Date: Tue, 14 Oct 2025 14:53:01 +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 1/2] media: cx18: Fix invalid access to file *
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@...aro.org>
> Closes: https://lore.kernel.org/all/aKL4OMWsESUdX8KQ@stanley.mountain/
> Fixes: 7b9eb53e8591 ("media: cx18: Access v4l2_fh from file")
> Cc: stable@...r.kernel.org
> Link: https://lore.kernel.org/stable/20250818-cx18-v4l2-fh-v1-1-6fe153760bce%40ideasonboard.com
> 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>
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);
>
Powered by blists - more mailing lists