linux-kernel - Re: [PATCH 1/3] media: subdev: Drop implicit zeroing of stream field

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <rzjtzsvk5q5oobh3khtjopn6ssqyf6akw2z3rswihfec3s3syw@xwjamnaqhplh>
Date:   Mon, 17 Jul 2023 16:11:50 +0200
From:   Jacopo Mondi <jacopo.mondi@...asonboard.com>
To:     Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc:     linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Jacopo Mondi <jacopo.mondi@...asonboard.com>
Subject: Re: [PATCH 1/3] media: subdev: Drop implicit zeroing of stream field

Hi Tomi

On Mon, Jun 19, 2023 at 02:27:05PM +0300, Tomi Valkeinen wrote:
> Now that the kernel drivers have been fixed to initialize the stream
> field, and we have the client capability which the userspace uses to say

Not sure I got this. Isn't the capabilities flag intended for drivers
to tell userspace it support streams ? This seems to suggest it is
userspace setting it ?

> it has initialized the stream field, we can drop the implicit zeroing of
> the stream field in the various check functions.
>

I guess this is safe, but I'm not sure why it wasn't before. If a
driver doesn't support streams (!(sd->flags & V4L2_SUBDEV_FL_STREAMS))
then it should have ignored the 'stream' field even if it wasn't
zeroed. So I suspect I am missing the reason for zeroing in first
place...

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 15 ---------------
>  1 file changed, 15 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 2ec179cd1264..c1ac6d7a63d2 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -200,9 +200,6 @@ static inline int check_format(struct v4l2_subdev *sd,
>  	if (!format)
>  		return -EINVAL;
>
> -	if (!(sd->flags & V4L2_SUBDEV_FL_STREAMS))
> -		format->stream = 0;
> -
>  	return check_which(format->which) ? : check_pad(sd, format->pad) ? :
>  	       check_state(sd, state, format->which, format->pad, format->stream);
>  }
> @@ -230,9 +227,6 @@ static int call_enum_mbus_code(struct v4l2_subdev *sd,
>  	if (!code)
>  		return -EINVAL;
>
> -	if (!(sd->flags & V4L2_SUBDEV_FL_STREAMS))
> -		code->stream = 0;
> -
>  	return check_which(code->which) ? : check_pad(sd, code->pad) ? :
>  	       check_state(sd, state, code->which, code->pad, code->stream) ? :
>  	       sd->ops->pad->enum_mbus_code(sd, state, code);
> @@ -245,9 +239,6 @@ static int call_enum_frame_size(struct v4l2_subdev *sd,
>  	if (!fse)
>  		return -EINVAL;
>
> -	if (!(sd->flags & V4L2_SUBDEV_FL_STREAMS))
> -		fse->stream = 0;
> -
>  	return check_which(fse->which) ? : check_pad(sd, fse->pad) ? :
>  	       check_state(sd, state, fse->which, fse->pad, fse->stream) ? :
>  	       sd->ops->pad->enum_frame_size(sd, state, fse);
> @@ -283,9 +274,6 @@ static int call_enum_frame_interval(struct v4l2_subdev *sd,
>  	if (!fie)
>  		return -EINVAL;
>
> -	if (!(sd->flags & V4L2_SUBDEV_FL_STREAMS))
> -		fie->stream = 0;
> -
>  	return check_which(fie->which) ? : check_pad(sd, fie->pad) ? :
>  	       check_state(sd, state, fie->which, fie->pad, fie->stream) ? :
>  	       sd->ops->pad->enum_frame_interval(sd, state, fie);
> @@ -298,9 +286,6 @@ static inline int check_selection(struct v4l2_subdev *sd,
>  	if (!sel)
>  		return -EINVAL;
>
> -	if (!(sd->flags & V4L2_SUBDEV_FL_STREAMS))
> -		sel->stream = 0;
> -
>  	return check_which(sel->which) ? : check_pad(sd, sel->pad) ? :
>  	       check_state(sd, state, sel->which, sel->pad, sel->stream);
>  }
> --
> 2.34.1
>