2019-05-14 22:53:40

by Janusz Krzysztofik

[permalink] [raw]
Subject: [PATCH v6 0/3] media: v4l2-subdev: Verify arguments in v4l2_subdev_call()

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.

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

Janusz Krzysztofik (3):
media: v4l2-subdev: Verify arguments in v4l2_subdev_call()
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 | 262 +++++++++++++++++---------
include/media/v4l2-subdev.h | 6 +
2 files changed, 182 insertions(+), 86 deletions(-)

Changelog:
v5->v6:
- rename wrappers to call_something() as suggested by Sakari - thanks!
- make check_ functions inline - also on Sakari's suggestion, thanks!
- drop patch 2/4 and remove WARN_ONs from remaining patches to avoid
kernel WARNs on non-kernel bugs - thanks Hans for pointing this out!

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


2019-05-14 22:54:38

by Janusz Krzysztofik

[permalink] [raw]
Subject: [PATCH v6 3/3] media: v4l2-subdev: Verify v4l2_subdev_call() pad config argument

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 <[email protected]>
---
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 6933f30e5041..6a5c4f046723 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -138,20 +138,30 @@ static inline int check_pad(struct v4l2_subdev *sd, __u32 pad)
return 0;
}

+static int check_cfg(__u32 which, struct v4l2_subdev_pad_config *cfg)
+{
+ if (which == V4L2_SUBDEV_FORMAT_TRY && !cfg)
+ return -EINVAL;
+
+ return 0;
+}
+
static inline int check_format(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *format)
{
if (!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 call_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 call_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 call_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 call_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 call_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 inline int check_selection(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_selection *sel)
{
if (!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 call_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 call_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