* [PATCH] media: v4l: ioctl: Sanitize num_planes before using it
@ 2019-02-18 10:25 Ezequiel Garcia
2019-02-18 11:38 ` Sakari Ailus
2019-02-18 15:14 ` Hans Verkuil
0 siblings, 2 replies; 3+ messages in thread
From: Ezequiel Garcia @ 2019-02-18 10:25 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, kernel, Ezequiel Garcia
The linked commit changed s_fmt/try_fmt to fail if num_planes is bogus.
This, however, is against the spec, which mandates drivers
to return a proper num_planes value, without an error.
Replace the num_planes check and instead clamp it to a sane value,
so we still make sure we don't overflow the planes array by accident.
Fixes: 9048b2e15b11c5 ("media: v4l: ioctl: Validate num_planes before using it")
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
drivers/media/v4l2-core/v4l2-ioctl.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 90aad465f9ed..206b7348797e 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1017,6 +1017,12 @@ static void v4l_sanitize_format(struct v4l2_format *fmt)
{
unsigned int offset;
+ /* Make sure num_planes is not bogus */
+ if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ||
+ fmt->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+ fmt->fmt.pix_mp.num_planes = min_t(u32, fmt->fmt.pix_mp.num_planes,
+ VIDEO_MAX_PLANES);
+
/*
* The v4l2_pix_format structure has been extended with fields that were
* not previously required to be set to zero by applications. The priv
@@ -1553,8 +1559,6 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
if (unlikely(!ops->vidioc_s_fmt_vid_cap_mplane))
break;
CLEAR_AFTER_FIELD(p, fmt.pix_mp.xfer_func);
- if (p->fmt.pix_mp.num_planes > VIDEO_MAX_PLANES)
- break;
for (i = 0; i < p->fmt.pix_mp.num_planes; i++)
CLEAR_AFTER_FIELD(&p->fmt.pix_mp.plane_fmt[i],
bytesperline);
@@ -1586,8 +1590,6 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
if (unlikely(!ops->vidioc_s_fmt_vid_out_mplane))
break;
CLEAR_AFTER_FIELD(p, fmt.pix_mp.xfer_func);
- if (p->fmt.pix_mp.num_planes > VIDEO_MAX_PLANES)
- break;
for (i = 0; i < p->fmt.pix_mp.num_planes; i++)
CLEAR_AFTER_FIELD(&p->fmt.pix_mp.plane_fmt[i],
bytesperline);
@@ -1656,8 +1658,6 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
if (unlikely(!ops->vidioc_try_fmt_vid_cap_mplane))
break;
CLEAR_AFTER_FIELD(p, fmt.pix_mp.xfer_func);
- if (p->fmt.pix_mp.num_planes > VIDEO_MAX_PLANES)
- break;
for (i = 0; i < p->fmt.pix_mp.num_planes; i++)
CLEAR_AFTER_FIELD(&p->fmt.pix_mp.plane_fmt[i],
bytesperline);
@@ -1689,8 +1689,6 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
if (unlikely(!ops->vidioc_try_fmt_vid_out_mplane))
break;
CLEAR_AFTER_FIELD(p, fmt.pix_mp.xfer_func);
- if (p->fmt.pix_mp.num_planes > VIDEO_MAX_PLANES)
- break;
for (i = 0; i < p->fmt.pix_mp.num_planes; i++)
CLEAR_AFTER_FIELD(&p->fmt.pix_mp.plane_fmt[i],
bytesperline);
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] media: v4l: ioctl: Sanitize num_planes before using it
2019-02-18 10:25 [PATCH] media: v4l: ioctl: Sanitize num_planes before using it Ezequiel Garcia
@ 2019-02-18 11:38 ` Sakari Ailus
2019-02-18 15:14 ` Hans Verkuil
1 sibling, 0 replies; 3+ messages in thread
From: Sakari Ailus @ 2019-02-18 11:38 UTC (permalink / raw)
To: Ezequiel Garcia; +Cc: linux-media, Hans Verkuil, kernel
Hi Ezequiel,
On Mon, Feb 18, 2019 at 07:25:42AM -0300, Ezequiel Garcia wrote:
> The linked commit changed s_fmt/try_fmt to fail if num_planes is bogus.
> This, however, is against the spec, which mandates drivers
> to return a proper num_planes value, without an error.
>
> Replace the num_planes check and instead clamp it to a sane value,
> so we still make sure we don't overflow the planes array by accident.
>
> Fixes: 9048b2e15b11c5 ("media: v4l: ioctl: Validate num_planes before using it")
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
--
Sakari Ailus
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] media: v4l: ioctl: Sanitize num_planes before using it
2019-02-18 10:25 [PATCH] media: v4l: ioctl: Sanitize num_planes before using it Ezequiel Garcia
2019-02-18 11:38 ` Sakari Ailus
@ 2019-02-18 15:14 ` Hans Verkuil
1 sibling, 0 replies; 3+ messages in thread
From: Hans Verkuil @ 2019-02-18 15:14 UTC (permalink / raw)
To: Ezequiel Garcia, linux-media; +Cc: Hans Verkuil, kernel
On 2/18/19 11:25 AM, Ezequiel Garcia wrote:
> The linked commit changed s_fmt/try_fmt to fail if num_planes is bogus.
> This, however, is against the spec, which mandates drivers
> to return a proper num_planes value, without an error.
>
> Replace the num_planes check and instead clamp it to a sane value,
> so we still make sure we don't overflow the planes array by accident.
>
> Fixes: 9048b2e15b11c5 ("media: v4l: ioctl: Validate num_planes before using it")
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Thanks!
Hans
> ---
> drivers/media/v4l2-core/v4l2-ioctl.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 90aad465f9ed..206b7348797e 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1017,6 +1017,12 @@ static void v4l_sanitize_format(struct v4l2_format *fmt)
> {
> unsigned int offset;
>
> + /* Make sure num_planes is not bogus */
> + if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ||
> + fmt->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> + fmt->fmt.pix_mp.num_planes = min_t(u32, fmt->fmt.pix_mp.num_planes,
> + VIDEO_MAX_PLANES);
> +
> /*
> * The v4l2_pix_format structure has been extended with fields that were
> * not previously required to be set to zero by applications. The priv
> @@ -1553,8 +1559,6 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
> if (unlikely(!ops->vidioc_s_fmt_vid_cap_mplane))
> break;
> CLEAR_AFTER_FIELD(p, fmt.pix_mp.xfer_func);
> - if (p->fmt.pix_mp.num_planes > VIDEO_MAX_PLANES)
> - break;
> for (i = 0; i < p->fmt.pix_mp.num_planes; i++)
> CLEAR_AFTER_FIELD(&p->fmt.pix_mp.plane_fmt[i],
> bytesperline);
> @@ -1586,8 +1590,6 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
> if (unlikely(!ops->vidioc_s_fmt_vid_out_mplane))
> break;
> CLEAR_AFTER_FIELD(p, fmt.pix_mp.xfer_func);
> - if (p->fmt.pix_mp.num_planes > VIDEO_MAX_PLANES)
> - break;
> for (i = 0; i < p->fmt.pix_mp.num_planes; i++)
> CLEAR_AFTER_FIELD(&p->fmt.pix_mp.plane_fmt[i],
> bytesperline);
> @@ -1656,8 +1658,6 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
> if (unlikely(!ops->vidioc_try_fmt_vid_cap_mplane))
> break;
> CLEAR_AFTER_FIELD(p, fmt.pix_mp.xfer_func);
> - if (p->fmt.pix_mp.num_planes > VIDEO_MAX_PLANES)
> - break;
> for (i = 0; i < p->fmt.pix_mp.num_planes; i++)
> CLEAR_AFTER_FIELD(&p->fmt.pix_mp.plane_fmt[i],
> bytesperline);
> @@ -1689,8 +1689,6 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
> if (unlikely(!ops->vidioc_try_fmt_vid_out_mplane))
> break;
> CLEAR_AFTER_FIELD(p, fmt.pix_mp.xfer_func);
> - if (p->fmt.pix_mp.num_planes > VIDEO_MAX_PLANES)
> - break;
> for (i = 0; i < p->fmt.pix_mp.num_planes; i++)
> CLEAR_AFTER_FIELD(&p->fmt.pix_mp.plane_fmt[i],
> bytesperline);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-02-18 15:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-18 10:25 [PATCH] media: v4l: ioctl: Sanitize num_planes before using it Ezequiel Garcia
2019-02-18 11:38 ` Sakari Ailus
2019-02-18 15:14 ` Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).