Hi Sakari,
Thanks for the patch.
On Monday 20 February 2012 03:56:45 Sakari Ailus wrote:
> Unify functions to get try pointers and validate the pad number accessed by
> the user.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxx>
> ---
> include/media/v4l2-subdev.h | 31 ++++++++++++++-----------------
> 1 files changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index bcaf6b8..d48dae5 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -565,23 +565,20 @@ struct v4l2_subdev_fh {
> container_of(fh, struct v4l2_subdev_fh, vfh)
>
> #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
> -static inline struct v4l2_mbus_framefmt *
> -v4l2_subdev_get_try_format(struct v4l2_subdev_fh *fh, unsigned int pad)
> -{
> - return &fh->pad[pad].try_fmt;
> -}
> -
> -static inline struct v4l2_rect *
> -v4l2_subdev_get_try_crop(struct v4l2_subdev_fh *fh, unsigned int pad)
> -{
> - return &fh->pad[pad].try_crop;
> -}
> -
> -static inline struct v4l2_rect *
> -v4l2_subdev_get_try_compose(struct v4l2_subdev_fh *fh, unsigned int pad)
> -{
> - return &fh->pad[pad].try_compose;
> -}
> +#define __V4L2_SUBDEV_MK_GET_TRY(rtype, fun_name, field_name) \
> + static inline struct rtype * \
> + v4l2_subdev_get_try_##fun_name(struct v4l2_subdev_fh *fh, \
> + unsigned int pad) \
> + { \
> + if (unlikely(pad > vdev_to_v4l2_subdev( \
> + fh->vfh.vdev->entity.num_pads) \
> + return NULL; \
> + return &fh->pad[pad].field_name; \
> + }
> +
> +__V4L2_SUBDEV_MK_GET_TRY(v4l2_mbus_framefmt, format, try_fmt)
> +__V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, crop, try_compose)
> +__V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, compose, try_compose)
> #endif
>
> extern const struct v4l2_file_operations v4l2_subdev_fops;
I'm not sure if this is a good idea. Drivers usually access the active and try
formats/rectangles through a single function that checks the which argument
and returns the active format/rectangle from the driver-specific device
structure, or calls v4l2_subdev_get_try_*. The pad number should be checked
for both active and try formats/rectangles, as both can result in accessing a
wrong memory location.
Furthermore, only in-kernel access to the active/try formats/rectangles need
to be checked, as the pad argument to subdev ioctls are already checked in
v4l2-subdev.c. If your goal is to catch buggy kernel code here, a BUG_ON might
be more suitable (although accessing the NULL pointer would result in an oops
anyway).
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
