* [PATCHv4 for v3.17] v4l2-ioctl: don't set PRIV_MAGIC unconditionally in g_fmt()
@ 2014-07-21 10:50 Hans Verkuil
2014-07-21 10:52 ` Laurent Pinchart
0 siblings, 1 reply; 2+ messages in thread
From: Hans Verkuil @ 2014-07-21 10:50 UTC (permalink / raw)
To: Linux Media Mailing List, Laurent Pinchart
Regression fix:
V4L2_PIX_FMT_PRIV_MAGIC should only be set for the VIDEO_CAPTURE and
VIDEO_OUTPUT buffer types, and not for any others. In the case of
the win format this overwrote a pointer value that is passed in from
userspace.
Just set it for V4L2_BUF_TYPE_VIDEO_CAPTURE and OUTPUT only. Set
it before the callback is called, just as is done for try/s_fmt, and
again afterwards in case the driver zeroed it. The latter was missing
in try/s_fmt, so add it there as well. Currently it is quite likely
that drivers clear priv (that was needed for a long time), so it makes
sense to set it twice.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index e620387..9fc8076 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1143,8 +1143,6 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
int ret;
- p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
-
/*
* fmt can't be cleared for these overlay types due to the 'clips'
* 'clipcount' and 'bitmap' pointers in struct v4l2_window.
@@ -1173,7 +1171,9 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
case V4L2_BUF_TYPE_VIDEO_CAPTURE:
if (unlikely(!is_rx || !is_vid || !ops->vidioc_g_fmt_vid_cap))
break;
+ p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
ret = ops->vidioc_g_fmt_vid_cap(file, fh, arg);
+ /* just in case the driver zeroed it again */
p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
return ret;
case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
@@ -1195,7 +1195,9 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
case V4L2_BUF_TYPE_VIDEO_OUTPUT:
if (unlikely(!is_tx || !is_vid || !ops->vidioc_g_fmt_vid_out))
break;
+ p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
ret = ops->vidioc_g_fmt_vid_out(file, fh, arg);
+ /* just in case the driver zeroed it again */
p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
return ret;
case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
@@ -1231,6 +1233,7 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
+ int ret;
v4l_sanitize_format(p);
@@ -1239,7 +1242,10 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
if (unlikely(!is_rx || !is_vid || !ops->vidioc_s_fmt_vid_cap))
break;
CLEAR_AFTER_FIELD(p, fmt.pix);
- return ops->vidioc_s_fmt_vid_cap(file, fh, arg);
+ ret = ops->vidioc_s_fmt_vid_cap(file, fh, arg);
+ /* just in case the driver zeroed it again */
+ p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
+ return ret;
case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
if (unlikely(!is_rx || !is_vid || !ops->vidioc_s_fmt_vid_cap_mplane))
break;
@@ -1264,7 +1270,10 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
if (unlikely(!is_tx || !is_vid || !ops->vidioc_s_fmt_vid_out))
break;
CLEAR_AFTER_FIELD(p, fmt.pix);
- return ops->vidioc_s_fmt_vid_out(file, fh, arg);
+ ret = ops->vidioc_s_fmt_vid_out(file, fh, arg);
+ /* just in case the driver zeroed it again */
+ p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
+ return ret;
case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
if (unlikely(!is_tx || !is_vid || !ops->vidioc_s_fmt_vid_out_mplane))
break;
@@ -1303,6 +1312,7 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
+ int ret;
v4l_sanitize_format(p);
@@ -1311,7 +1321,10 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
if (unlikely(!is_rx || !is_vid || !ops->vidioc_try_fmt_vid_cap))
break;
CLEAR_AFTER_FIELD(p, fmt.pix);
- return ops->vidioc_try_fmt_vid_cap(file, fh, arg);
+ ret = ops->vidioc_try_fmt_vid_cap(file, fh, arg);
+ /* just in case the driver zeroed it again */
+ p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
+ return ret;
case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
if (unlikely(!is_rx || !is_vid || !ops->vidioc_try_fmt_vid_cap_mplane))
break;
@@ -1336,7 +1349,10 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops,
if (unlikely(!is_tx || !is_vid || !ops->vidioc_try_fmt_vid_out))
break;
CLEAR_AFTER_FIELD(p, fmt.pix);
- return ops->vidioc_try_fmt_vid_out(file, fh, arg);
+ ret = ops->vidioc_try_fmt_vid_out(file, fh, arg);
+ /* just in case the driver zeroed it again */
+ p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
+ return ret;
case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
if (unlikely(!is_tx || !is_vid || !ops->vidioc_try_fmt_vid_out_mplane))
break;
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCHv4 for v3.17] v4l2-ioctl: don't set PRIV_MAGIC unconditionally in g_fmt()
2014-07-21 10:50 [PATCHv4 for v3.17] v4l2-ioctl: don't set PRIV_MAGIC unconditionally in g_fmt() Hans Verkuil
@ 2014-07-21 10:52 ` Laurent Pinchart
0 siblings, 0 replies; 2+ messages in thread
From: Laurent Pinchart @ 2014-07-21 10:52 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Linux Media Mailing List
Hi Hans,
Thank you for the patch.
On Monday 21 July 2014 12:50:39 Hans Verkuil wrote:
> Regression fix:
>
> V4L2_PIX_FMT_PRIV_MAGIC should only be set for the VIDEO_CAPTURE and
> VIDEO_OUTPUT buffer types, and not for any others. In the case of
> the win format this overwrote a pointer value that is passed in from
> userspace.
>
> Just set it for V4L2_BUF_TYPE_VIDEO_CAPTURE and OUTPUT only. Set
> it before the callback is called, just as is done for try/s_fmt, and
> again afterwards in case the driver zeroed it. The latter was missing
> in try/s_fmt, so add it there as well. Currently it is quite likely
> that drivers clear priv (that was needed for a long time), so it makes
> sense to set it twice.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c index e620387..9fc8076 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1143,8 +1143,6 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
> bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
> int ret;
>
> - p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> -
> /*
> * fmt can't be cleared for these overlay types due to the 'clips'
> * 'clipcount' and 'bitmap' pointers in struct v4l2_window.
> @@ -1173,7 +1171,9 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
> case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> if (unlikely(!is_rx || !is_vid || !ops->vidioc_g_fmt_vid_cap))
> break;
> + p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> ret = ops->vidioc_g_fmt_vid_cap(file, fh, arg);
> + /* just in case the driver zeroed it again */
> p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> return ret;
> case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> @@ -1195,7 +1195,9 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
> case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> if (unlikely(!is_tx || !is_vid || !ops->vidioc_g_fmt_vid_out))
> break;
> + p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> ret = ops->vidioc_g_fmt_vid_out(file, fh, arg);
> + /* just in case the driver zeroed it again */
> p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> return ret;
> case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> @@ -1231,6 +1233,7 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
> bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
> bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
> bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
> + int ret;
>
> v4l_sanitize_format(p);
>
> @@ -1239,7 +1242,10 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops
> *ops, if (unlikely(!is_rx || !is_vid || !ops->vidioc_s_fmt_vid_cap))
> break;
> CLEAR_AFTER_FIELD(p, fmt.pix);
> - return ops->vidioc_s_fmt_vid_cap(file, fh, arg);
> + ret = ops->vidioc_s_fmt_vid_cap(file, fh, arg);
> + /* just in case the driver zeroed it again */
> + p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> + return ret;
> case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> if (unlikely(!is_rx || !is_vid || !ops->vidioc_s_fmt_vid_cap_mplane))
> break;
> @@ -1264,7 +1270,10 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops
> *ops, if (unlikely(!is_tx || !is_vid || !ops->vidioc_s_fmt_vid_out))
> break;
> CLEAR_AFTER_FIELD(p, fmt.pix);
> - return ops->vidioc_s_fmt_vid_out(file, fh, arg);
> + ret = ops->vidioc_s_fmt_vid_out(file, fh, arg);
> + /* just in case the driver zeroed it again */
> + p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> + return ret;
> case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> if (unlikely(!is_tx || !is_vid || !ops->vidioc_s_fmt_vid_out_mplane))
> break;
> @@ -1303,6 +1312,7 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops
> *ops, bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
> bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
> bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
> + int ret;
>
> v4l_sanitize_format(p);
>
> @@ -1311,7 +1321,10 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops
> *ops, if (unlikely(!is_rx || !is_vid || !ops->vidioc_try_fmt_vid_cap))
> break;
> CLEAR_AFTER_FIELD(p, fmt.pix);
> - return ops->vidioc_try_fmt_vid_cap(file, fh, arg);
> + ret = ops->vidioc_try_fmt_vid_cap(file, fh, arg);
> + /* just in case the driver zeroed it again */
> + p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> + return ret;
> case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> if (unlikely(!is_rx || !is_vid || !ops-
>vidioc_try_fmt_vid_cap_mplane))
> break;
> @@ -1336,7 +1349,10 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops
> *ops, if (unlikely(!is_tx || !is_vid || !ops->vidioc_try_fmt_vid_out))
> break;
> CLEAR_AFTER_FIELD(p, fmt.pix);
> - return ops->vidioc_try_fmt_vid_out(file, fh, arg);
> + ret = ops->vidioc_try_fmt_vid_out(file, fh, arg);
> + /* just in case the driver zeroed it again */
> + p->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> + return ret;
> case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> if (unlikely(!is_tx || !is_vid || !ops-
>vidioc_try_fmt_vid_out_mplane))
> break;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-07-21 10:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-21 10:50 [PATCHv4 for v3.17] v4l2-ioctl: don't set PRIV_MAGIC unconditionally in g_fmt() Hans Verkuil
2014-07-21 10:52 ` Laurent Pinchart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox