[PATCH v5 0/4] media: v4l2-subdev: Verify arguments in v4l2_subdev_call()

[PATCH v5 0/4] media: v4l2-subdev: Verify arguments in v4l2_subdev_call()

[Janusz Krzysztofik]

Correctness of format type (try or active) and pad ID parameters passed
to subdevice operation callbacks is now verified only for IOCTL calls.
However, those callbacks are also used by drivers, e.g., V4L2 host
interfaces.
    
Since both subdev_do_ioctl() and drivers are using v4l2_subdev_call()
macro while calling subdevice operations, move those parameter checks
from subdev_do_ioctl() to v4l2_subdev_call().  Also, add check for
non-NULL pointers, including pad config if V4L2_SUBDEV_FORMAT_tRY is
requested.  Since invalid arguments generally mean bugs, be noisy about
them.

Having that done, we can avoid taking care of those checks inside
drivers.

Janusz Krzysztofik (4):
  media: v4l2-subdev: Verify arguments in v4l2_subdev_call()
  media: v4l2-subdev: WARN_ON invalid v4l2_subdev_call() arguments
  media: v4l2-subdev: Verify v4l2_subdev_call() pointer arguments
  media: v4l2-subdev: Verify v4l2_subdev_call() pad config argument

 drivers/media/v4l2-core/v4l2-subdev.c | 257 +++++++++++++++++---------
 include/media/v4l2-subdev.h           |   6 +
 2 files changed, 179 insertions(+), 84 deletions(-)

Changelog:
v4->v5:
- a few coding style and code formatting changes,
- require CONFIG_MEDIA_CONTROLLER, not CONFIG_VIDEO_V4L2_SUBDEV_API,
  for a valid pad ID check,
- perform pad ID check only if at least one pad is configured so
  drivers which don't configure pads are not affected if built with
  CONFIG_MEDIA_CONTROLLER defined,
- issue kernel warnings on invalid parameters (new patch - 2/4),
- validate pointers before using them (new patch - 3/4).

v3->v4:
- fix 'struct' keyword missing from patch 2/2,
- fix checkpatch reported style issue in patch 2/2
Sorry for that.

v2->v3:
- add patch 2/2 with pad config check,
- adjust continuation line alignments in patch 1/2 to match those
  used in 2/2.

v1->v2:
- replace the horrible macro with a structure of wrapper functions;
  inspired by Hans' and Sakari's comments - thanks!

-- 
2.21.0

[PATCH v5 1/4] media: v4l2-subdev: Verify arguments in v4l2_subdev_call()

[Janusz Krzysztofik]

Correctness of format type (try or active) and pad ID parameters passed
to subdevice operation callbacks is now verified only for IOCTL calls.
However, those callbacks are also used by drivers, e.g., V4L2 host
interfaces.

Since both subdev_do_ioctl() and drivers are using v4l2_subdev_call()
macro while calling subdevice operations, move those parameter checks
from subdev_do_ioctl() to v4l2_subdev_call() so we can avoid taking care
of those checks inside drivers.

Define a wrapper function for each operation callback in scope, then
gather those wrappers in a static v4l2_subdev_ops structure so the
v4l2_subdev_call() macro can find them easy if provided.  Move reusable
code to helpers.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 227 ++++++++++++++++----------
 include/media/v4l2-subdev.h           |   6 +
 2 files changed, 143 insertions(+), 90 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index d75815ab0d7b..890916674d42 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -120,56 +120,168 @@ static int subdev_close(struct file *file)
 	return 0;
 }
 
-#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
-static int check_format(struct v4l2_subdev *sd,
-			struct v4l2_subdev_format *format)
+static int check_which(__u32 which)
 {
-	if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
-	    format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
-		return -EINVAL;
-
-	if (format->pad >= sd->entity.num_pads)
+	if (which != V4L2_SUBDEV_FORMAT_TRY &&
+	    which != V4L2_SUBDEV_FORMAT_ACTIVE)
 		return -EINVAL;
 
 	return 0;
 }
 
-static int check_crop(struct v4l2_subdev *sd, struct v4l2_subdev_crop *crop)
+static int check_pad(struct v4l2_subdev *sd, __u32 pad)
 {
-	if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
-	    crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+#if defined(CONFIG_MEDIA_CONTROLLER)
+	if (sd->entity.num_pads && pad >= sd->entity.num_pads)
 		return -EINVAL;
+#endif
+	return 0;
+}
 
-	if (crop->pad >= sd->entity.num_pads)
-		return -EINVAL;
+static int check_format(struct v4l2_subdev *sd,
+			struct v4l2_subdev_format *format)
+{
+	return check_which(format->which) ? : check_pad(sd, format->pad);
+}
 
-	return 0;
+static int check_get_fmt(struct v4l2_subdev *sd,
+			 struct v4l2_subdev_pad_config *cfg,
+			 struct v4l2_subdev_format *format)
+{
+	return check_format(sd, format) ? :
+	       sd->ops->pad->get_fmt(sd, cfg, format);
+}
+
+static int check_set_fmt(struct v4l2_subdev *sd,
+			 struct v4l2_subdev_pad_config *cfg,
+			 struct v4l2_subdev_format *format)
+{
+	return check_format(sd, format) ? :
+	       sd->ops->pad->set_fmt(sd, cfg, format);
+}
+
+static int check_enum_mbus_code(struct v4l2_subdev *sd,
+				struct v4l2_subdev_pad_config *cfg,
+				struct v4l2_subdev_mbus_code_enum *code)
+{
+	return check_which(code->which) ? : check_pad(sd, code->pad) ? :
+	       sd->ops->pad->enum_mbus_code(sd, cfg, code);
+}
+
+static int check_enum_frame_size(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_pad_config *cfg,
+				 struct v4l2_subdev_frame_size_enum *fse)
+{
+	return check_which(fse->which) ? : check_pad(sd, fse->pad) ? :
+	       sd->ops->pad->enum_frame_size(sd, cfg, fse);
+}
+
+static int check_frame_interval(struct v4l2_subdev *sd,
+				struct v4l2_subdev_frame_interval *fi)
+{
+	return check_pad(sd, fi->pad);
+}
+
+static int check_g_frame_interval(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_frame_interval *fi)
+{
+	return check_frame_interval(sd, fi) ? :
+	       sd->ops->video->g_frame_interval(sd, fi);
+}
+
+static int check_s_frame_interval(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_frame_interval *fi)
+{
+	return check_frame_interval(sd, fi) ? :
+	       sd->ops->video->s_frame_interval(sd, fi);
+}
+
+static int check_enum_frame_interval(struct v4l2_subdev *sd,
+				struct v4l2_subdev_pad_config *cfg,
+				struct v4l2_subdev_frame_interval_enum *fie)
+{
+	return check_which(fie->which) ? : check_pad(sd, fie->pad) ? :
+	       sd->ops->pad->enum_frame_interval(sd, cfg, fie);
 }
 
 static int check_selection(struct v4l2_subdev *sd,
 			   struct v4l2_subdev_selection *sel)
 {
-	if (sel->which != V4L2_SUBDEV_FORMAT_TRY &&
-	    sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
-		return -EINVAL;
+	return check_which(sel->which) ? : check_pad(sd, sel->pad);
+}
 
-	if (sel->pad >= sd->entity.num_pads)
-		return -EINVAL;
+static int check_get_selection(struct v4l2_subdev *sd,
+			       struct v4l2_subdev_pad_config *cfg,
+			       struct v4l2_subdev_selection *sel)
+{
+	return check_selection(sd, sel) ? :
+	       sd->ops->pad->get_selection(sd, cfg, sel);
+}
 
-	return 0;
+static int check_set_selection(struct v4l2_subdev *sd,
+			       struct v4l2_subdev_pad_config *cfg,
+			       struct v4l2_subdev_selection *sel)
+{
+	return check_selection(sd, sel) ? :
+	       sd->ops->pad->set_selection(sd, cfg, sel);
 }
 
 static int check_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid)
 {
-	if (edid->pad >= sd->entity.num_pads)
-		return -EINVAL;
-
 	if (edid->blocks && edid->edid == NULL)
 		return -EINVAL;
 
-	return 0;
+	return check_pad(sd, edid->pad);
 }
-#endif
+
+static int check_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid)
+{
+	return check_edid(sd, edid) ? : sd->ops->pad->get_edid(sd, edid);
+}
+
+static int check_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid)
+{
+	return check_edid(sd, edid) ? : sd->ops->pad->set_edid(sd, edid);
+}
+
+static int check_dv_timings_cap(struct v4l2_subdev *sd,
+				struct v4l2_dv_timings_cap *cap)
+{
+	return check_pad(sd, cap->pad) ? :
+	       sd->ops->pad->dv_timings_cap(sd, cap);
+}
+
+static int check_enum_dv_timings(struct v4l2_subdev *sd,
+				 struct v4l2_enum_dv_timings *dvt)
+{
+	return check_pad(sd, dvt->pad) ? :
+	       sd->ops->pad->enum_dv_timings(sd, dvt);
+}
+
+static const struct v4l2_subdev_pad_ops v4l2_subdev_pad_chk_args = {
+	.get_fmt		= check_get_fmt,
+	.set_fmt		= check_set_fmt,
+	.enum_mbus_code		= check_enum_mbus_code,
+	.enum_frame_size	= check_enum_frame_size,
+	.enum_frame_interval	= check_enum_frame_interval,
+	.get_selection		= check_get_selection,
+	.set_selection		= check_set_selection,
+	.get_edid		= check_get_edid,
+	.set_edid		= check_set_edid,
+	.dv_timings_cap		= check_dv_timings_cap,
+	.enum_dv_timings	= check_enum_dv_timings,
+};
+
+static const struct v4l2_subdev_video_ops v4l2_subdev_video_chk_args = {
+	.g_frame_interval	= check_g_frame_interval,
+	.s_frame_interval	= check_s_frame_interval,
+};
+
+struct v4l2_subdev_ops v4l2_subdev_call_chk_args = {
+	.pad	= &v4l2_subdev_pad_chk_args,
+	.video	= &v4l2_subdev_video_chk_args,
+};
+EXPORT_SYMBOL(v4l2_subdev_call_chk_args);
 
 static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 {
@@ -292,10 +404,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_SUBDEV_G_FMT: {
 		struct v4l2_subdev_format *format = arg;
 
-		rval = check_format(sd, format);
-		if (rval)
-			return rval;
-
 		memset(format->reserved, 0, sizeof(format->reserved));
 		memset(format->format.reserved, 0, sizeof(format->format.reserved));
 		return v4l2_subdev_call(sd, pad, get_fmt, subdev_fh->pad, format);
@@ -304,10 +412,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_SUBDEV_S_FMT: {
 		struct v4l2_subdev_format *format = arg;
 
-		rval = check_format(sd, format);
-		if (rval)
-			return rval;
-
 		memset(format->reserved, 0, sizeof(format->reserved));
 		memset(format->format.reserved, 0, sizeof(format->format.reserved));
 		return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format);
@@ -317,10 +421,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		struct v4l2_subdev_crop *crop = arg;
 		struct v4l2_subdev_selection sel;
 
-		rval = check_crop(sd, crop);
-		if (rval)
-			return rval;
-
 		memset(crop->reserved, 0, sizeof(crop->reserved));
 		memset(&sel, 0, sizeof(sel));
 		sel.which = crop->which;
@@ -340,10 +440,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 		struct v4l2_subdev_selection sel;
 
 		memset(crop->reserved, 0, sizeof(crop->reserved));
-		rval = check_crop(sd, crop);
-		if (rval)
-			return rval;
-
 		memset(&sel, 0, sizeof(sel));
 		sel.which = crop->which;
 		sel.pad = crop->pad;
@@ -361,13 +457,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_SUBDEV_ENUM_MBUS_CODE: {
 		struct v4l2_subdev_mbus_code_enum *code = arg;
 
-		if (code->which != V4L2_SUBDEV_FORMAT_TRY &&
-		    code->which != V4L2_SUBDEV_FORMAT_ACTIVE)
-			return -EINVAL;
-
-		if (code->pad >= sd->entity.num_pads)
-			return -EINVAL;
-
 		memset(code->reserved, 0, sizeof(code->reserved));
 		return v4l2_subdev_call(sd, pad, enum_mbus_code, subdev_fh->pad,
 					code);
@@ -376,13 +465,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_SUBDEV_ENUM_FRAME_SIZE: {
 		struct v4l2_subdev_frame_size_enum *fse = arg;
 
-		if (fse->which != V4L2_SUBDEV_FORMAT_TRY &&
-		    fse->which != V4L2_SUBDEV_FORMAT_ACTIVE)
-			return -EINVAL;
-
-		if (fse->pad >= sd->entity.num_pads)
-			return -EINVAL;
-
 		memset(fse->reserved, 0, sizeof(fse->reserved));
 		return v4l2_subdev_call(sd, pad, enum_frame_size, subdev_fh->pad,
 					fse);
@@ -391,9 +473,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_SUBDEV_G_FRAME_INTERVAL: {
 		struct v4l2_subdev_frame_interval *fi = arg;
 
-		if (fi->pad >= sd->entity.num_pads)
-			return -EINVAL;
-
 		memset(fi->reserved, 0, sizeof(fi->reserved));
 		return v4l2_subdev_call(sd, video, g_frame_interval, arg);
 	}
@@ -401,9 +480,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_SUBDEV_S_FRAME_INTERVAL: {
 		struct v4l2_subdev_frame_interval *fi = arg;
 
-		if (fi->pad >= sd->entity.num_pads)
-			return -EINVAL;
-
 		memset(fi->reserved, 0, sizeof(fi->reserved));
 		return v4l2_subdev_call(sd, video, s_frame_interval, arg);
 	}
@@ -411,13 +487,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL: {
 		struct v4l2_subdev_frame_interval_enum *fie = arg;
 
-		if (fie->which != V4L2_SUBDEV_FORMAT_TRY &&
-		    fie->which != V4L2_SUBDEV_FORMAT_ACTIVE)
-			return -EINVAL;
-
-		if (fie->pad >= sd->entity.num_pads)
-			return -EINVAL;
-
 		memset(fie->reserved, 0, sizeof(fie->reserved));
 		return v4l2_subdev_call(sd, pad, enum_frame_interval, subdev_fh->pad,
 					fie);
@@ -426,10 +495,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_SUBDEV_G_SELECTION: {
 		struct v4l2_subdev_selection *sel = arg;
 
-		rval = check_selection(sd, sel);
-		if (rval)
-			return rval;
-
 		memset(sel->reserved, 0, sizeof(sel->reserved));
 		return v4l2_subdev_call(
 			sd, pad, get_selection, subdev_fh->pad, sel);
@@ -438,10 +503,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_SUBDEV_S_SELECTION: {
 		struct v4l2_subdev_selection *sel = arg;
 
-		rval = check_selection(sd, sel);
-		if (rval)
-			return rval;
-
 		memset(sel->reserved, 0, sizeof(sel->reserved));
 		return v4l2_subdev_call(
 			sd, pad, set_selection, subdev_fh->pad, sel);
@@ -450,38 +511,24 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	case VIDIOC_G_EDID: {
 		struct v4l2_subdev_edid *edid = arg;
 
-		rval = check_edid(sd, edid);
-		if (rval)
-			return rval;
-
 		return v4l2_subdev_call(sd, pad, get_edid, edid);
 	}
 
 	case VIDIOC_S_EDID: {
 		struct v4l2_subdev_edid *edid = arg;
 
-		rval = check_edid(sd, edid);
-		if (rval)
-			return rval;
-
 		return v4l2_subdev_call(sd, pad, set_edid, edid);
 	}
 
 	case VIDIOC_SUBDEV_DV_TIMINGS_CAP: {
 		struct v4l2_dv_timings_cap *cap = arg;
 
-		if (cap->pad >= sd->entity.num_pads)
-			return -EINVAL;
-
 		return v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
 	}
 
 	case VIDIOC_SUBDEV_ENUM_DV_TIMINGS: {
 		struct v4l2_enum_dv_timings *dvt = arg;
 
-		if (dvt->pad >= sd->entity.num_pads)
-			return -EINVAL;
-
 		return v4l2_subdev_call(sd, pad, enum_dv_timings, dvt);
 	}
 
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index a7fa5b80915a..900f265fd950 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -1091,6 +1091,8 @@ void v4l2_subdev_free_pad_config(struct v4l2_subdev_pad_config *cfg);
 void v4l2_subdev_init(struct v4l2_subdev *sd,
 		      const struct v4l2_subdev_ops *ops);
 
+extern struct v4l2_subdev_ops v4l2_subdev_call_chk_args;
+
 /**
  * v4l2_subdev_call - call an operation of a v4l2_subdev.
  *
@@ -1112,6 +1114,10 @@ void v4l2_subdev_init(struct v4l2_subdev *sd,
 			__result = -ENODEV;				\
 		else if (!(__sd->ops->o && __sd->ops->o->f))		\
 			__result = -ENOIOCTLCMD;			\
+		else if (v4l2_subdev_call_chk_args.o &&			\
+			 v4l2_subdev_call_chk_args.o->f)		\
+			__result = v4l2_subdev_call_chk_args.o->f(	\
+							__sd, ##args);	\
 		else							\
 			__result = __sd->ops->o->f(__sd, ##args);	\
 		__result;						\
-- 
2.21.0

[PATCH v5 2/4] media: v4l2-subdev: WARN_ON invalid v4l2_subdev_call() arguments

[Janusz Krzysztofik]

Invalid arguments passed to v4l2_subdev_call generally mean bugs.  Be
noisy if that happens.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 890916674d42..5f2264575cd7 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -122,8 +122,8 @@ static int subdev_close(struct file *file)
 
 static int check_which(__u32 which)
 {
-	if (which != V4L2_SUBDEV_FORMAT_TRY &&
-	    which != V4L2_SUBDEV_FORMAT_ACTIVE)
+	if (WARN_ON(which != V4L2_SUBDEV_FORMAT_TRY &&
+		    which != V4L2_SUBDEV_FORMAT_ACTIVE))
 		return -EINVAL;
 
 	return 0;
@@ -132,7 +132,7 @@ static int check_which(__u32 which)
 static int check_pad(struct v4l2_subdev *sd, __u32 pad)
 {
 #if defined(CONFIG_MEDIA_CONTROLLER)
-	if (sd->entity.num_pads && pad >= sd->entity.num_pads)
+	if (WARN_ON(sd->entity.num_pads && pad >= sd->entity.num_pads))
 		return -EINVAL;
 #endif
 	return 0;
@@ -228,7 +228,7 @@ static int check_set_selection(struct v4l2_subdev *sd,
 
 static int check_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid)
 {
-	if (edid->blocks && edid->edid == NULL)
+	if (WARN_ON(edid->blocks && edid->edid == NULL))
 		return -EINVAL;
 
 	return check_pad(sd, edid->pad);
-- 
2.21.0

[PATCH v5 3/4] media: v4l2-subdev: Verify v4l2_subdev_call() pointer arguments

[Janusz Krzysztofik]

Parameters passed to check helpers are now obtained by dereferencing
unverified pointer arguments.  Check validity of those pointers first.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 5f2264575cd7..3fc07af26c5b 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -141,6 +141,9 @@ static int check_pad(struct v4l2_subdev *sd, __u32 pad)
 static int check_format(struct v4l2_subdev *sd,
 			struct v4l2_subdev_format *format)
 {
+	if (WARN_ON(!format))
+		return -EINVAL;
+
 	return check_which(format->which) ? : check_pad(sd, format->pad);
 }
 
@@ -164,6 +167,9 @@ static int check_enum_mbus_code(struct v4l2_subdev *sd,
 				struct v4l2_subdev_pad_config *cfg,
 				struct v4l2_subdev_mbus_code_enum *code)
 {
+	if (WARN_ON(!code))
+		return -EINVAL;
+
 	return check_which(code->which) ? : check_pad(sd, code->pad) ? :
 	       sd->ops->pad->enum_mbus_code(sd, cfg, code);
 }
@@ -172,6 +178,9 @@ static int check_enum_frame_size(struct v4l2_subdev *sd,
 				 struct v4l2_subdev_pad_config *cfg,
 				 struct v4l2_subdev_frame_size_enum *fse)
 {
+	if (WARN_ON(!fse))
+		return -EINVAL;
+
 	return check_which(fse->which) ? : check_pad(sd, fse->pad) ? :
 	       sd->ops->pad->enum_frame_size(sd, cfg, fse);
 }
@@ -179,6 +188,9 @@ static int check_enum_frame_size(struct v4l2_subdev *sd,
 static int check_frame_interval(struct v4l2_subdev *sd,
 				struct v4l2_subdev_frame_interval *fi)
 {
+	if (WARN_ON(!fi))
+		return -EINVAL;
+
 	return check_pad(sd, fi->pad);
 }
 
@@ -200,6 +212,9 @@ static int check_enum_frame_interval(struct v4l2_subdev *sd,
 				struct v4l2_subdev_pad_config *cfg,
 				struct v4l2_subdev_frame_interval_enum *fie)
 {
+	if (WARN_ON(!fie))
+		return -EINVAL;
+
 	return check_which(fie->which) ? : check_pad(sd, fie->pad) ? :
 	       sd->ops->pad->enum_frame_interval(sd, cfg, fie);
 }
@@ -207,6 +222,9 @@ static int check_enum_frame_interval(struct v4l2_subdev *sd,
 static int check_selection(struct v4l2_subdev *sd,
 			   struct v4l2_subdev_selection *sel)
 {
+	if (WARN_ON(!sel))
+		return -EINVAL;
+
 	return check_which(sel->which) ? : check_pad(sd, sel->pad);
 }
 
@@ -228,6 +246,9 @@ static int check_set_selection(struct v4l2_subdev *sd,
 
 static int check_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid)
 {
+	if (WARN_ON(!edid))
+		return -EINVAL;
+
 	if (WARN_ON(edid->blocks && edid->edid == NULL))
 		return -EINVAL;
 
@@ -247,6 +268,9 @@ static int check_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid)
 static int check_dv_timings_cap(struct v4l2_subdev *sd,
 				struct v4l2_dv_timings_cap *cap)
 {
+	if (WARN_ON(!cap))
+		return -EINVAL;
+
 	return check_pad(sd, cap->pad) ? :
 	       sd->ops->pad->dv_timings_cap(sd, cap);
 }
@@ -254,6 +278,9 @@ static int check_dv_timings_cap(struct v4l2_subdev *sd,
 static int check_enum_dv_timings(struct v4l2_subdev *sd,
 				 struct v4l2_enum_dv_timings *dvt)
 {
+	if (WARN_ON(!dvt))
+		return -EINVAL;
+
 	return check_pad(sd, dvt->pad) ? :
 	       sd->ops->pad->enum_dv_timings(sd, dvt);
 }
-- 
2.21.0

[PATCH v5 4/4] media: v4l2-subdev: Verify v4l2_subdev_call() pad config argument

[Janusz Krzysztofik]

Extend parameter checks performed by v4l2_subdev_call() with a check for
a non-NULL pad config pointer if V4L2_SUBDEV_FORMAT_TRY format type is
requested so drivers don't need to care.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 3fc07af26c5b..fc8c308fb060 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -138,20 +138,30 @@ static int check_pad(struct v4l2_subdev *sd, __u32 pad)
 	return 0;
 }
 
+static int check_cfg(__u32 which, struct v4l2_subdev_pad_config *cfg)
+{
+	if (WARN_ON(which == V4L2_SUBDEV_FORMAT_TRY && !cfg))
+		return -EINVAL;
+
+	return 0;
+}
+
 static int check_format(struct v4l2_subdev *sd,
+			struct v4l2_subdev_pad_config *cfg,
 			struct v4l2_subdev_format *format)
 {
 	if (WARN_ON(!format))
 		return -EINVAL;
 
-	return check_which(format->which) ? : check_pad(sd, format->pad);
+	return check_which(format->which) ? : check_pad(sd, format->pad) ? :
+	       check_cfg(format->which, cfg);
 }
 
 static int check_get_fmt(struct v4l2_subdev *sd,
 			 struct v4l2_subdev_pad_config *cfg,
 			 struct v4l2_subdev_format *format)
 {
-	return check_format(sd, format) ? :
+	return check_format(sd, cfg, format) ? :
 	       sd->ops->pad->get_fmt(sd, cfg, format);
 }
 
@@ -159,7 +169,7 @@ static int check_set_fmt(struct v4l2_subdev *sd,
 			 struct v4l2_subdev_pad_config *cfg,
 			 struct v4l2_subdev_format *format)
 {
-	return check_format(sd, format) ? :
+	return check_format(sd, cfg, format) ? :
 	       sd->ops->pad->set_fmt(sd, cfg, format);
 }
 
@@ -171,6 +181,7 @@ static int check_enum_mbus_code(struct v4l2_subdev *sd,
 		return -EINVAL;
 
 	return check_which(code->which) ? : check_pad(sd, code->pad) ? :
+	       check_cfg(code->which, cfg) ? :
 	       sd->ops->pad->enum_mbus_code(sd, cfg, code);
 }
 
@@ -182,6 +193,7 @@ static int check_enum_frame_size(struct v4l2_subdev *sd,
 		return -EINVAL;
 
 	return check_which(fse->which) ? : check_pad(sd, fse->pad) ? :
+	       check_cfg(fse->which, cfg) ? :
 	       sd->ops->pad->enum_frame_size(sd, cfg, fse);
 }
 
@@ -216,23 +228,26 @@ static int check_enum_frame_interval(struct v4l2_subdev *sd,
 		return -EINVAL;
 
 	return check_which(fie->which) ? : check_pad(sd, fie->pad) ? :
+	       check_cfg(fie->which, cfg) ? :
 	       sd->ops->pad->enum_frame_interval(sd, cfg, fie);
 }
 
 static int check_selection(struct v4l2_subdev *sd,
+			   struct v4l2_subdev_pad_config *cfg,
 			   struct v4l2_subdev_selection *sel)
 {
 	if (WARN_ON(!sel))
 		return -EINVAL;
 
-	return check_which(sel->which) ? : check_pad(sd, sel->pad);
+	return check_which(sel->which) ? : check_pad(sd, sel->pad) ? :
+	       check_cfg(sel->which, cfg);
 }
 
 static int check_get_selection(struct v4l2_subdev *sd,
 			       struct v4l2_subdev_pad_config *cfg,
 			       struct v4l2_subdev_selection *sel)
 {
-	return check_selection(sd, sel) ? :
+	return check_selection(sd, cfg, sel) ? :
 	       sd->ops->pad->get_selection(sd, cfg, sel);
 }
 
@@ -240,7 +255,7 @@ static int check_set_selection(struct v4l2_subdev *sd,
 			       struct v4l2_subdev_pad_config *cfg,
 			       struct v4l2_subdev_selection *sel)
 {
-	return check_selection(sd, sel) ? :
+	return check_selection(sd, cfg, sel) ? :
 	       sd->ops->pad->set_selection(sd, cfg, sel);
 }
 
-- 
2.21.0

Re: [PATCH v5 2/4] media: v4l2-subdev: WARN_ON invalid v4l2_subdev_call() arguments

[Hans Verkuil]

On 5/12/19 10:15 PM, Janusz Krzysztofik wrote:
> Invalid arguments passed to v4l2_subdev_call generally mean bugs.  Be
> noisy if that happens.
> 
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 890916674d42..5f2264575cd7 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -122,8 +122,8 @@ static int subdev_close(struct file *file)
>  
>  static int check_which(__u32 which)
>  {
> -	if (which != V4L2_SUBDEV_FORMAT_TRY &&
> -	    which != V4L2_SUBDEV_FORMAT_ACTIVE)
> +	if (WARN_ON(which != V4L2_SUBDEV_FORMAT_TRY &&
> +		    which != V4L2_SUBDEV_FORMAT_ACTIVE))
>  		return -EINVAL;

But this is now also called when the user calls an ioctl. And in that
case there should be no warning. It's perfectly legal for userspace to
specify a random value for which, and in that case it is just a regular
userspace bug. Same for the other checks below.

Regards,

	Hans

>  
>  	return 0;
> @@ -132,7 +132,7 @@ static int check_which(__u32 which)
>  static int check_pad(struct v4l2_subdev *sd, __u32 pad)
>  {
>  #if defined(CONFIG_MEDIA_CONTROLLER)
> -	if (sd->entity.num_pads && pad >= sd->entity.num_pads)
> +	if (WARN_ON(sd->entity.num_pads && pad >= sd->entity.num_pads))
>  		return -EINVAL;
>  #endif
>  	return 0;
> @@ -228,7 +228,7 @@ static int check_set_selection(struct v4l2_subdev *sd,
>  
>  static int check_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid)
>  {
> -	if (edid->blocks && edid->edid == NULL)
> +	if (WARN_ON(edid->blocks && edid->edid == NULL))
>  		return -EINVAL;
>  
>  	return check_pad(sd, edid->pad);
> 

Re: [PATCH v5 1/4] media: v4l2-subdev: Verify arguments in v4l2_subdev_call()

[Hans Verkuil]

On 5/12/19 10:15 PM, Janusz Krzysztofik wrote:
> Correctness of format type (try or active) and pad ID parameters passed
> to subdevice operation callbacks is now verified only for IOCTL calls.
> However, those callbacks are also used by drivers, e.g., V4L2 host
> interfaces.
> 
> Since both subdev_do_ioctl() and drivers are using v4l2_subdev_call()
> macro while calling subdevice operations, move those parameter checks
> from subdev_do_ioctl() to v4l2_subdev_call() so we can avoid taking care
> of those checks inside drivers.
> 
> Define a wrapper function for each operation callback in scope, then
> gather those wrappers in a static v4l2_subdev_ops structure so the
> v4l2_subdev_call() macro can find them easy if provided.  Move reusable
> code to helpers.
> 
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 227 ++++++++++++++++----------
>  include/media/v4l2-subdev.h           |   6 +
>  2 files changed, 143 insertions(+), 90 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index d75815ab0d7b..890916674d42 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -120,56 +120,168 @@ static int subdev_close(struct file *file)
>  	return 0;
>  }
>  
> -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
> -static int check_format(struct v4l2_subdev *sd,
> -			struct v4l2_subdev_format *format)
> +static int check_which(__u32 which)
>  {
> -	if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
> -	    format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> -		return -EINVAL;
> -
> -	if (format->pad >= sd->entity.num_pads)
> +	if (which != V4L2_SUBDEV_FORMAT_TRY &&
> +	    which != V4L2_SUBDEV_FORMAT_ACTIVE)
>  		return -EINVAL;
>  
>  	return 0;
>  }
>  
> -static int check_crop(struct v4l2_subdev *sd, struct v4l2_subdev_crop *crop)
> +static int check_pad(struct v4l2_subdev *sd, __u32 pad)
>  {
> -	if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
> -	    crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +	if (sd->entity.num_pads && pad >= sd->entity.num_pads)
>  		return -EINVAL;
> +#endif
> +	return 0;
> +}
>  
> -	if (crop->pad >= sd->entity.num_pads)
> -		return -EINVAL;
> +static int check_format(struct v4l2_subdev *sd,
> +			struct v4l2_subdev_format *format)
> +{
> +	return check_which(format->which) ? : check_pad(sd, format->pad);
> +}
>  
> -	return 0;
> +static int check_get_fmt(struct v4l2_subdev *sd,
> +			 struct v4l2_subdev_pad_config *cfg,
> +			 struct v4l2_subdev_format *format)
> +{
> +	return check_format(sd, format) ? :
> +	       sd->ops->pad->get_fmt(sd, cfg, format);
> +}
> +
> +static int check_set_fmt(struct v4l2_subdev *sd,
> +			 struct v4l2_subdev_pad_config *cfg,
> +			 struct v4l2_subdev_format *format)
> +{
> +	return check_format(sd, format) ? :
> +	       sd->ops->pad->set_fmt(sd, cfg, format);
> +}
> +
> +static int check_enum_mbus_code(struct v4l2_subdev *sd,
> +				struct v4l2_subdev_pad_config *cfg,
> +				struct v4l2_subdev_mbus_code_enum *code)
> +{
> +	return check_which(code->which) ? : check_pad(sd, code->pad) ? :
> +	       sd->ops->pad->enum_mbus_code(sd, cfg, code);
> +}
> +
> +static int check_enum_frame_size(struct v4l2_subdev *sd,
> +				 struct v4l2_subdev_pad_config *cfg,
> +				 struct v4l2_subdev_frame_size_enum *fse)
> +{
> +	return check_which(fse->which) ? : check_pad(sd, fse->pad) ? :
> +	       sd->ops->pad->enum_frame_size(sd, cfg, fse);
> +}
> +
> +static int check_frame_interval(struct v4l2_subdev *sd,
> +				struct v4l2_subdev_frame_interval *fi)
> +{
> +	return check_pad(sd, fi->pad);
> +}
> +
> +static int check_g_frame_interval(struct v4l2_subdev *sd,
> +				  struct v4l2_subdev_frame_interval *fi)
> +{
> +	return check_frame_interval(sd, fi) ? :
> +	       sd->ops->video->g_frame_interval(sd, fi);
> +}
> +
> +static int check_s_frame_interval(struct v4l2_subdev *sd,
> +				  struct v4l2_subdev_frame_interval *fi)
> +{
> +	return check_frame_interval(sd, fi) ? :
> +	       sd->ops->video->s_frame_interval(sd, fi);
> +}
> +
> +static int check_enum_frame_interval(struct v4l2_subdev *sd,
> +				struct v4l2_subdev_pad_config *cfg,
> +				struct v4l2_subdev_frame_interval_enum *fie)
> +{
> +	return check_which(fie->which) ? : check_pad(sd, fie->pad) ? :
> +	       sd->ops->pad->enum_frame_interval(sd, cfg, fie);
>  }
>  
>  static int check_selection(struct v4l2_subdev *sd,
>  			   struct v4l2_subdev_selection *sel)
>  {
> -	if (sel->which != V4L2_SUBDEV_FORMAT_TRY &&
> -	    sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> -		return -EINVAL;
> +	return check_which(sel->which) ? : check_pad(sd, sel->pad);
> +}
>  
> -	if (sel->pad >= sd->entity.num_pads)
> -		return -EINVAL;
> +static int check_get_selection(struct v4l2_subdev *sd,
> +			       struct v4l2_subdev_pad_config *cfg,
> +			       struct v4l2_subdev_selection *sel)
> +{
> +	return check_selection(sd, sel) ? :
> +	       sd->ops->pad->get_selection(sd, cfg, sel);
> +}
>  
> -	return 0;
> +static int check_set_selection(struct v4l2_subdev *sd,
> +			       struct v4l2_subdev_pad_config *cfg,
> +			       struct v4l2_subdev_selection *sel)
> +{
> +	return check_selection(sd, sel) ? :
> +	       sd->ops->pad->set_selection(sd, cfg, sel);
>  }
>  
>  static int check_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid)
>  {
> -	if (edid->pad >= sd->entity.num_pads)
> -		return -EINVAL;
> -
>  	if (edid->blocks && edid->edid == NULL)
>  		return -EINVAL;
>  
> -	return 0;
> +	return check_pad(sd, edid->pad);
>  }
> -#endif
> +
> +static int check_get_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid)
> +{
> +	return check_edid(sd, edid) ? : sd->ops->pad->get_edid(sd, edid);
> +}
> +
> +static int check_set_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid)
> +{
> +	return check_edid(sd, edid) ? : sd->ops->pad->set_edid(sd, edid);
> +}
> +
> +static int check_dv_timings_cap(struct v4l2_subdev *sd,
> +				struct v4l2_dv_timings_cap *cap)
> +{
> +	return check_pad(sd, cap->pad) ? :
> +	       sd->ops->pad->dv_timings_cap(sd, cap);
> +}
> +
> +static int check_enum_dv_timings(struct v4l2_subdev *sd,
> +				 struct v4l2_enum_dv_timings *dvt)
> +{
> +	return check_pad(sd, dvt->pad) ? :
> +	       sd->ops->pad->enum_dv_timings(sd, dvt);
> +}
> +
> +static const struct v4l2_subdev_pad_ops v4l2_subdev_pad_chk_args = {
> +	.get_fmt		= check_get_fmt,
> +	.set_fmt		= check_set_fmt,
> +	.enum_mbus_code		= check_enum_mbus_code,
> +	.enum_frame_size	= check_enum_frame_size,
> +	.enum_frame_interval	= check_enum_frame_interval,
> +	.get_selection		= check_get_selection,
> +	.set_selection		= check_set_selection,
> +	.get_edid		= check_get_edid,
> +	.set_edid		= check_set_edid,
> +	.dv_timings_cap		= check_dv_timings_cap,
> +	.enum_dv_timings	= check_enum_dv_timings,
> +};
> +
> +static const struct v4l2_subdev_video_ops v4l2_subdev_video_chk_args = {
> +	.g_frame_interval	= check_g_frame_interval,
> +	.s_frame_interval	= check_s_frame_interval,
> +};
> +
> +struct v4l2_subdev_ops v4l2_subdev_call_chk_args = {

Should be const.

BTW, I agree with Sakari regarding the naming of these ops.

Regards,

	Hans

> +	.pad	= &v4l2_subdev_pad_chk_args,
> +	.video	= &v4l2_subdev_video_chk_args,
> +};
> +EXPORT_SYMBOL(v4l2_subdev_call_chk_args);
>  
>  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  {
> @@ -292,10 +404,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	case VIDIOC_SUBDEV_G_FMT: {
>  		struct v4l2_subdev_format *format = arg;
>  
> -		rval = check_format(sd, format);
> -		if (rval)
> -			return rval;
> -
>  		memset(format->reserved, 0, sizeof(format->reserved));
>  		memset(format->format.reserved, 0, sizeof(format->format.reserved));
>  		return v4l2_subdev_call(sd, pad, get_fmt, subdev_fh->pad, format);
> @@ -304,10 +412,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	case VIDIOC_SUBDEV_S_FMT: {
>  		struct v4l2_subdev_format *format = arg;
>  
> -		rval = check_format(sd, format);
> -		if (rval)
> -			return rval;
> -
>  		memset(format->reserved, 0, sizeof(format->reserved));
>  		memset(format->format.reserved, 0, sizeof(format->format.reserved));
>  		return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format);
> @@ -317,10 +421,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  		struct v4l2_subdev_crop *crop = arg;
>  		struct v4l2_subdev_selection sel;
>  
> -		rval = check_crop(sd, crop);
> -		if (rval)
> -			return rval;
> -
>  		memset(crop->reserved, 0, sizeof(crop->reserved));
>  		memset(&sel, 0, sizeof(sel));
>  		sel.which = crop->which;
> @@ -340,10 +440,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  		struct v4l2_subdev_selection sel;
>  
>  		memset(crop->reserved, 0, sizeof(crop->reserved));
> -		rval = check_crop(sd, crop);
> -		if (rval)
> -			return rval;
> -
>  		memset(&sel, 0, sizeof(sel));
>  		sel.which = crop->which;
>  		sel.pad = crop->pad;
> @@ -361,13 +457,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	case VIDIOC_SUBDEV_ENUM_MBUS_CODE: {
>  		struct v4l2_subdev_mbus_code_enum *code = arg;
>  
> -		if (code->which != V4L2_SUBDEV_FORMAT_TRY &&
> -		    code->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> -			return -EINVAL;
> -
> -		if (code->pad >= sd->entity.num_pads)
> -			return -EINVAL;
> -
>  		memset(code->reserved, 0, sizeof(code->reserved));
>  		return v4l2_subdev_call(sd, pad, enum_mbus_code, subdev_fh->pad,
>  					code);
> @@ -376,13 +465,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	case VIDIOC_SUBDEV_ENUM_FRAME_SIZE: {
>  		struct v4l2_subdev_frame_size_enum *fse = arg;
>  
> -		if (fse->which != V4L2_SUBDEV_FORMAT_TRY &&
> -		    fse->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> -			return -EINVAL;
> -
> -		if (fse->pad >= sd->entity.num_pads)
> -			return -EINVAL;
> -
>  		memset(fse->reserved, 0, sizeof(fse->reserved));
>  		return v4l2_subdev_call(sd, pad, enum_frame_size, subdev_fh->pad,
>  					fse);
> @@ -391,9 +473,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	case VIDIOC_SUBDEV_G_FRAME_INTERVAL: {
>  		struct v4l2_subdev_frame_interval *fi = arg;
>  
> -		if (fi->pad >= sd->entity.num_pads)
> -			return -EINVAL;
> -
>  		memset(fi->reserved, 0, sizeof(fi->reserved));
>  		return v4l2_subdev_call(sd, video, g_frame_interval, arg);
>  	}
> @@ -401,9 +480,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	case VIDIOC_SUBDEV_S_FRAME_INTERVAL: {
>  		struct v4l2_subdev_frame_interval *fi = arg;
>  
> -		if (fi->pad >= sd->entity.num_pads)
> -			return -EINVAL;
> -
>  		memset(fi->reserved, 0, sizeof(fi->reserved));
>  		return v4l2_subdev_call(sd, video, s_frame_interval, arg);
>  	}
> @@ -411,13 +487,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	case VIDIOC_SUBDEV_ENUM_FRAME_INTERVAL: {
>  		struct v4l2_subdev_frame_interval_enum *fie = arg;
>  
> -		if (fie->which != V4L2_SUBDEV_FORMAT_TRY &&
> -		    fie->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> -			return -EINVAL;
> -
> -		if (fie->pad >= sd->entity.num_pads)
> -			return -EINVAL;
> -
>  		memset(fie->reserved, 0, sizeof(fie->reserved));
>  		return v4l2_subdev_call(sd, pad, enum_frame_interval, subdev_fh->pad,
>  					fie);
> @@ -426,10 +495,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	case VIDIOC_SUBDEV_G_SELECTION: {
>  		struct v4l2_subdev_selection *sel = arg;
>  
> -		rval = check_selection(sd, sel);
> -		if (rval)
> -			return rval;
> -
>  		memset(sel->reserved, 0, sizeof(sel->reserved));
>  		return v4l2_subdev_call(
>  			sd, pad, get_selection, subdev_fh->pad, sel);
> @@ -438,10 +503,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	case VIDIOC_SUBDEV_S_SELECTION: {
>  		struct v4l2_subdev_selection *sel = arg;
>  
> -		rval = check_selection(sd, sel);
> -		if (rval)
> -			return rval;
> -
>  		memset(sel->reserved, 0, sizeof(sel->reserved));
>  		return v4l2_subdev_call(
>  			sd, pad, set_selection, subdev_fh->pad, sel);
> @@ -450,38 +511,24 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
>  	case VIDIOC_G_EDID: {
>  		struct v4l2_subdev_edid *edid = arg;
>  
> -		rval = check_edid(sd, edid);
> -		if (rval)
> -			return rval;
> -
>  		return v4l2_subdev_call(sd, pad, get_edid, edid);
>  	}
>  
>  	case VIDIOC_S_EDID: {
>  		struct v4l2_subdev_edid *edid = arg;
>  
> -		rval = check_edid(sd, edid);
> -		if (rval)
> -			return rval;
> -
>  		return v4l2_subdev_call(sd, pad, set_edid, edid);
>  	}
>  
>  	case VIDIOC_SUBDEV_DV_TIMINGS_CAP: {
>  		struct v4l2_dv_timings_cap *cap = arg;
>  
> -		if (cap->pad >= sd->entity.num_pads)
> -			return -EINVAL;
> -
>  		return v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
>  	}
>  
>  	case VIDIOC_SUBDEV_ENUM_DV_TIMINGS: {
>  		struct v4l2_enum_dv_timings *dvt = arg;
>  
> -		if (dvt->pad >= sd->entity.num_pads)
> -			return -EINVAL;
> -
>  		return v4l2_subdev_call(sd, pad, enum_dv_timings, dvt);
>  	}
>  
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index a7fa5b80915a..900f265fd950 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -1091,6 +1091,8 @@ void v4l2_subdev_free_pad_config(struct v4l2_subdev_pad_config *cfg);
>  void v4l2_subdev_init(struct v4l2_subdev *sd,
>  		      const struct v4l2_subdev_ops *ops);
>  
> +extern struct v4l2_subdev_ops v4l2_subdev_call_chk_args;
> +
>  /**
>   * v4l2_subdev_call - call an operation of a v4l2_subdev.
>   *
> @@ -1112,6 +1114,10 @@ void v4l2_subdev_init(struct v4l2_subdev *sd,
>  			__result = -ENODEV;				\
>  		else if (!(__sd->ops->o && __sd->ops->o->f))		\
>  			__result = -ENOIOCTLCMD;			\
> +		else if (v4l2_subdev_call_chk_args.o &&			\
> +			 v4l2_subdev_call_chk_args.o->f)		\
> +			__result = v4l2_subdev_call_chk_args.o->f(	\
> +							__sd, ##args);	\
>  		else							\
>  			__result = __sd->ops->o->f(__sd, ##args);	\
>  		__result;						\
>