diff mbox series

[for,v6.3] media: v4l2-subdev.c: clear stream field

Message ID e9cb93fd-cdb7-7e67-ca79-1e3ffd1241e6@xs4all.nl (mailing list archive)
State New, archived
Headers show
Series [for,v6.3] media: v4l2-subdev.c: clear stream field | expand

Commit Message

Hans Verkuil Feb. 15, 2023, 2:48 p.m. UTC 
Both userspace and kernelspace can pass structs with an uninitialized 'stream'
field. Since the check_state() function checks for a non-zero stream field,
suddenly these calls will fails with -EINVAL.

So check in the wrapper functions in v4l2-subdev.c (which are used by both the
kernel and userspace API) if V4L2_SUBDEV_FL_STREAMS is set, and if not, then zero
the stream field.

Currently no drivers set V4L2_SUBDEV_FL_STREAMS.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---

Comments

Laurent Pinchart Feb. 15, 2023, 5 p.m. UTC | #1 
Hi Hans,

Thank you for the patch.

On Wed, Feb 15, 2023 at 03:48:17PM +0100, Hans Verkuil wrote:
> Both userspace and kernelspace can pass structs with an uninitialized 'stream'
> field. Since the check_state() function checks for a non-zero stream field,
> suddenly these calls will fails with -EINVAL.
> 
> So check in the wrapper functions in v4l2-subdev.c (which are used by both the
> kernel and userspace API) if V4L2_SUBDEV_FL_STREAMS is set, and if not, then zero
> the stream field.
> 
> Currently no drivers set V4L2_SUBDEV_FL_STREAMS.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Looks good to me as a v6.3 fix. I would however like to then revert it
in v6.4 once a better fix gets developed: if an application that does
support the streams API sets the stream field to an invalid value for a
subdev that doesn't support streams, this shouldn't be silently ignored.

Could you capture this in the commit message ?

> ---
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 1bebcda2bd20..db22da153002 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -197,6 +198,8 @@ static inline int check_format(struct v4l2_subdev *sd,
>  	if (!format)
>  		return -EINVAL;
> 
> +	if (!(sd->flags & V4L2_SUBDEV_FL_STREAMS))
> +		format->stream = 0;

Could you add a blank line here ? Same below. With that,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	return check_which(format->which) ? : check_pad(sd, format->pad) ? :
>  	       check_state(sd, state, format->which, format->pad, format->stream);
>  }
> @@ -224,6 +227,8 @@ 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);
> @@ -236,6 +241,8 @@ 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);
> @@ -271,6 +278,8 @@ 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);
> @@ -283,6 +292,8 @@ 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);
>  }
diff mbox series

Patch

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 1bebcda2bd20..db22da153002 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -197,6 +198,8 @@  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);
 }
@@ -224,6 +227,8 @@  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);
@@ -236,6 +241,8 @@  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);
@@ -271,6 +278,8 @@  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);
@@ -283,6 +292,8 @@  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);
 }