* [PATCHv6 0/3] v4l2-core: improve ioctl validation
@ 2019-10-14 8:40 Hans Verkuil
2019-10-14 8:40 ` [PATCHv6 1/3] v4l2-core: correctly validate video and metadata ioctls Hans Verkuil
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-10-14 8:40 UTC (permalink / raw)
To: linux-media; +Cc: Vandana BN, Sakari Ailus, Laurent Pinchart
This supersedes https://www.mail-archive.com/linux-media@vger.kernel.org/msg150027.html
based on feedback from Laurent:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg150117.html
and Sakari:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg150129.html
This v6 only moves some code from patch 1 to patch 3, the final code
is the same as for v5. I plan to make a PR for this very soon together
with the vivid metadata patches that need this.
Regards,
Hans
Changes in v6:
Patch 1/3 dropped the check against GRABBER for the g_parm ioctl,
but that is too early: this should be done in patch 3/3 where this
code no longer applies to touch devices (which was the reason for
the GRABBER test).
Changes in v5:
I now check if a GRABBER device is a video or metadata device
(or both!) by checking device_caps.
Hans Verkuil (2):
v4l2-dev: simplify the SDR checks
v4l2-dev: fix is_tch checks
Vandana BN (1):
v4l2-core: correctly validate video and metadata ioctls
drivers/media/v4l2-core/v4l2-dev.c | 104 ++++++++++++++++-----------
drivers/media/v4l2-core/v4l2-ioctl.c | 16 ++++-
2 files changed, 75 insertions(+), 45 deletions(-)
--
2.23.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCHv6 1/3] v4l2-core: correctly validate video and metadata ioctls
2019-10-14 8:40 [PATCHv6 0/3] v4l2-core: improve ioctl validation Hans Verkuil
@ 2019-10-14 8:40 ` Hans Verkuil
2019-10-14 21:36 ` Laurent Pinchart
2019-10-14 8:40 ` [PATCHv6 2/3] v4l2-dev: simplify the SDR checks Hans Verkuil
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2019-10-14 8:40 UTC (permalink / raw)
To: linux-media; +Cc: Vandana BN, Sakari Ailus, Laurent Pinchart, Hans Verkuil
From: Vandana BN <bnvandana@gmail.com>
If the type is VFL_TYPE_GRABBER, then also check device_caps
to see if the video device supports video and/or metadata and
disable unneeded ioctls.
Without this change, format ioctls for both video and metadata devices
could be called on both device nodes. This is true for other ioctls as
well, even if the device supports only video or metadata.
Metadata devices act similar to VBI devices w.r.t. which ioctls should
be enabled. This makes sense since VBI *is* metadata.
Signed-off-by: Vandana BN <bnvandana@gmail.com>
Co-Developed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/v4l2-core/v4l2-dev.c | 62 +++++++++++++++++-----------
drivers/media/v4l2-core/v4l2-ioctl.c | 16 +++++--
2 files changed, 52 insertions(+), 26 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 4037689a945a..1bf543932e4f 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -533,13 +533,23 @@ static int get_index(struct video_device *vdev)
*/
static void determine_valid_ioctls(struct video_device *vdev)
{
+ const u32 vid_caps = V4L2_CAP_VIDEO_CAPTURE |
+ V4L2_CAP_VIDEO_CAPTURE_MPLANE |
+ V4L2_CAP_VIDEO_OUTPUT |
+ V4L2_CAP_VIDEO_OUTPUT_MPLANE |
+ V4L2_CAP_VIDEO_M2M | V4L2_CAP_VIDEO_M2M_MPLANE;
+ const u32 meta_caps = V4L2_CAP_META_CAPTURE |
+ V4L2_CAP_META_OUTPUT;
DECLARE_BITMAP(valid_ioctls, BASE_VIDIOC_PRIVATE);
const struct v4l2_ioctl_ops *ops = vdev->ioctl_ops;
- bool is_vid = vdev->vfl_type == VFL_TYPE_GRABBER;
+ bool is_vid = vdev->vfl_type == VFL_TYPE_GRABBER &&
+ (vdev->device_caps & vid_caps);
bool is_vbi = vdev->vfl_type == VFL_TYPE_VBI;
bool is_radio = vdev->vfl_type == VFL_TYPE_RADIO;
bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
bool is_tch = vdev->vfl_type == VFL_TYPE_TOUCH;
+ bool is_meta = vdev->vfl_type == VFL_TYPE_GRABBER &&
+ (vdev->device_caps & meta_caps);
bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
@@ -587,39 +597,31 @@ static void determine_valid_ioctls(struct video_device *vdev)
set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
if (is_vid || is_tch) {
- /* video and metadata specific ioctls */
+ /* video and touch specific ioctls */
if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
- ops->vidioc_enum_fmt_vid_overlay ||
- ops->vidioc_enum_fmt_meta_cap)) ||
- (is_tx && (ops->vidioc_enum_fmt_vid_out ||
- ops->vidioc_enum_fmt_meta_out)))
+ ops->vidioc_enum_fmt_vid_overlay)) ||
+ (is_tx && ops->vidioc_enum_fmt_vid_out))
set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
if ((is_rx && (ops->vidioc_g_fmt_vid_cap ||
ops->vidioc_g_fmt_vid_cap_mplane ||
- ops->vidioc_g_fmt_vid_overlay ||
- ops->vidioc_g_fmt_meta_cap)) ||
+ ops->vidioc_g_fmt_vid_overlay)) ||
(is_tx && (ops->vidioc_g_fmt_vid_out ||
ops->vidioc_g_fmt_vid_out_mplane ||
- ops->vidioc_g_fmt_vid_out_overlay ||
- ops->vidioc_g_fmt_meta_out)))
+ ops->vidioc_g_fmt_vid_out_overlay)))
set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
if ((is_rx && (ops->vidioc_s_fmt_vid_cap ||
ops->vidioc_s_fmt_vid_cap_mplane ||
- ops->vidioc_s_fmt_vid_overlay ||
- ops->vidioc_s_fmt_meta_cap)) ||
+ ops->vidioc_s_fmt_vid_overlay)) ||
(is_tx && (ops->vidioc_s_fmt_vid_out ||
ops->vidioc_s_fmt_vid_out_mplane ||
- ops->vidioc_s_fmt_vid_out_overlay ||
- ops->vidioc_s_fmt_meta_out)))
+ ops->vidioc_s_fmt_vid_out_overlay)))
set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
if ((is_rx && (ops->vidioc_try_fmt_vid_cap ||
ops->vidioc_try_fmt_vid_cap_mplane ||
- ops->vidioc_try_fmt_vid_overlay ||
- ops->vidioc_try_fmt_meta_cap)) ||
+ ops->vidioc_try_fmt_vid_overlay)) ||
(is_tx && (ops->vidioc_try_fmt_vid_out ||
ops->vidioc_try_fmt_vid_out_mplane ||
- ops->vidioc_try_fmt_vid_out_overlay ||
- ops->vidioc_try_fmt_meta_out)))
+ ops->vidioc_try_fmt_vid_out_overlay)))
set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
@@ -641,7 +643,21 @@ static void determine_valid_ioctls(struct video_device *vdev)
set_bit(_IOC_NR(VIDIOC_S_CROP), valid_ioctls);
SET_VALID_IOCTL(ops, VIDIOC_G_SELECTION, vidioc_g_selection);
SET_VALID_IOCTL(ops, VIDIOC_S_SELECTION, vidioc_s_selection);
- } else if (is_vbi) {
+ }
+ if (is_meta && is_rx) {
+ /* metadata capture specific ioctls */
+ SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_meta_cap);
+ SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_meta_cap);
+ SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_meta_cap);
+ SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_meta_cap);
+ } else if (is_meta && is_tx) {
+ /* metadata output specific ioctls */
+ SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_meta_out);
+ SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_meta_out);
+ SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_meta_out);
+ SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_meta_out);
+ }
+ if (is_vbi) {
/* vbi specific ioctls */
if ((is_rx && (ops->vidioc_g_fmt_vbi_cap ||
ops->vidioc_g_fmt_sliced_vbi_cap)) ||
@@ -681,8 +697,8 @@ static void determine_valid_ioctls(struct video_device *vdev)
set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
}
- if (is_vid || is_vbi || is_sdr || is_tch) {
- /* ioctls valid for video, metadata, vbi or sdr */
+ if (is_vid || is_vbi || is_sdr || is_tch || is_meta) {
+ /* ioctls valid for video, vbi, sdr, touch and metadata */
SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
SET_VALID_IOCTL(ops, VIDIOC_QBUF, vidioc_qbuf);
@@ -694,8 +710,8 @@ static void determine_valid_ioctls(struct video_device *vdev)
SET_VALID_IOCTL(ops, VIDIOC_STREAMOFF, vidioc_streamoff);
}
- if (is_vid || is_vbi || is_tch) {
- /* ioctls valid for video or vbi */
+ if (is_vid || is_vbi || is_tch || is_meta) {
+ /* ioctls valid for video, vbi, touch and metadata */
if (ops->vidioc_s_std)
set_bit(_IOC_NR(VIDIOC_ENUMSTD), valid_ioctls);
SET_VALID_IOCTL(ops, VIDIOC_S_STD, vidioc_s_std);
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 51b912743f0f..20b3107dd4e8 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -932,12 +932,22 @@ static int check_ext_ctrls(struct v4l2_ext_controls *c, int allow_priv)
static int check_fmt(struct file *file, enum v4l2_buf_type type)
{
+ const u32 vid_caps = V4L2_CAP_VIDEO_CAPTURE |
+ V4L2_CAP_VIDEO_CAPTURE_MPLANE |
+ V4L2_CAP_VIDEO_OUTPUT |
+ V4L2_CAP_VIDEO_OUTPUT_MPLANE |
+ V4L2_CAP_VIDEO_M2M | V4L2_CAP_VIDEO_M2M_MPLANE;
+ const u32 meta_caps = V4L2_CAP_META_CAPTURE |
+ V4L2_CAP_META_OUTPUT;
struct video_device *vfd = video_devdata(file);
const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops;
- bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
+ bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER &&
+ (vfd->device_caps & vid_caps);
bool is_vbi = vfd->vfl_type == VFL_TYPE_VBI;
bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
bool is_tch = vfd->vfl_type == VFL_TYPE_TOUCH;
+ bool is_meta = vfd->vfl_type == VFL_TYPE_GRABBER &&
+ (vfd->device_caps & meta_caps);
bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
@@ -996,11 +1006,11 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
return 0;
break;
case V4L2_BUF_TYPE_META_CAPTURE:
- if (is_vid && is_rx && ops->vidioc_g_fmt_meta_cap)
+ if (is_meta && is_rx && ops->vidioc_g_fmt_meta_cap)
return 0;
break;
case V4L2_BUF_TYPE_META_OUTPUT:
- if (is_vid && is_tx && ops->vidioc_g_fmt_meta_out)
+ if (is_meta && is_tx && ops->vidioc_g_fmt_meta_out)
return 0;
break;
default:
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCHv6 2/3] v4l2-dev: simplify the SDR checks
2019-10-14 8:40 [PATCHv6 0/3] v4l2-core: improve ioctl validation Hans Verkuil
2019-10-14 8:40 ` [PATCHv6 1/3] v4l2-core: correctly validate video and metadata ioctls Hans Verkuil
@ 2019-10-14 8:40 ` Hans Verkuil
2019-10-14 21:39 ` Laurent Pinchart
2019-10-14 8:40 ` [PATCHv6 3/3] v4l2-dev: fix is_tch checks Hans Verkuil
2019-10-15 20:19 ` [PATCHv6 0/3] v4l2-core: improve ioctl validation Laurent Pinchart
3 siblings, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2019-10-14 8:40 UTC (permalink / raw)
To: linux-media; +Cc: Vandana BN, Sakari Ailus, Laurent Pinchart, Hans Verkuil
In determine_valid_ioctls() we can use SET_VALID_IOCTL to enable
ioctls for SDR, simplifying the code.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/v4l2-core/v4l2-dev.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 1bf543932e4f..27fb96a6c2a8 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -677,24 +677,16 @@ static void determine_valid_ioctls(struct video_device *vdev)
SET_VALID_IOCTL(ops, VIDIOC_G_SLICED_VBI_CAP, vidioc_g_sliced_vbi_cap);
} else if (is_sdr && is_rx) {
/* SDR receiver specific ioctls */
- if (ops->vidioc_enum_fmt_sdr_cap)
- set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
- if (ops->vidioc_g_fmt_sdr_cap)
- set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
- if (ops->vidioc_s_fmt_sdr_cap)
- set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
- if (ops->vidioc_try_fmt_sdr_cap)
- set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
+ SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_sdr_cap);
+ SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_sdr_cap);
+ SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_sdr_cap);
+ SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_sdr_cap);
} else if (is_sdr && is_tx) {
/* SDR transmitter specific ioctls */
- if (ops->vidioc_enum_fmt_sdr_out)
- set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
- if (ops->vidioc_g_fmt_sdr_out)
- set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
- if (ops->vidioc_s_fmt_sdr_out)
- set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
- if (ops->vidioc_try_fmt_sdr_out)
- set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
+ SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_sdr_out);
+ SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_sdr_out);
+ SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_sdr_out);
+ SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_sdr_out);
}
if (is_vid || is_vbi || is_sdr || is_tch || is_meta) {
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCHv6 3/3] v4l2-dev: fix is_tch checks
2019-10-14 8:40 [PATCHv6 0/3] v4l2-core: improve ioctl validation Hans Verkuil
2019-10-14 8:40 ` [PATCHv6 1/3] v4l2-core: correctly validate video and metadata ioctls Hans Verkuil
2019-10-14 8:40 ` [PATCHv6 2/3] v4l2-dev: simplify the SDR checks Hans Verkuil
@ 2019-10-14 8:40 ` Hans Verkuil
2019-10-14 12:01 ` [PATCHv6 4/3] v4l2-dev: disable frequency and tuner ioctls for touch Hans Verkuil
2019-10-14 21:42 ` [PATCHv6 3/3] v4l2-dev: fix is_tch checks Laurent Pinchart
2019-10-15 20:19 ` [PATCHv6 0/3] v4l2-core: improve ioctl validation Laurent Pinchart
3 siblings, 2 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-10-14 8:40 UTC (permalink / raw)
To: linux-media; +Cc: Vandana BN, Sakari Ailus, Laurent Pinchart, Hans Verkuil
Touch devices mark too many ioctls as valid. Restrict the list of
valid ioctls for touch devices.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/v4l2-core/v4l2-dev.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 27fb96a6c2a8..cec588b04711 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -596,8 +596,8 @@ static void determine_valid_ioctls(struct video_device *vdev)
if (ops->vidioc_enum_freq_bands || ops->vidioc_g_tuner || ops->vidioc_g_modulator)
set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
- if (is_vid || is_tch) {
- /* video and touch specific ioctls */
+ if (is_vid) {
+ /* video specific ioctls */
if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
ops->vidioc_enum_fmt_vid_overlay)) ||
(is_tx && ops->vidioc_enum_fmt_vid_out))
@@ -675,6 +675,19 @@ static void determine_valid_ioctls(struct video_device *vdev)
ops->vidioc_try_fmt_sliced_vbi_out)))
set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
SET_VALID_IOCTL(ops, VIDIOC_G_SLICED_VBI_CAP, vidioc_g_sliced_vbi_cap);
+ } else if (is_tch) {
+ /* touch specific ioctls */
+ SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_vid_cap);
+ SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_vid_cap);
+ SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_vid_cap);
+ SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_vid_cap);
+ SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMESIZES, vidioc_enum_framesizes);
+ SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMEINTERVALS, vidioc_enum_frameintervals);
+ SET_VALID_IOCTL(ops, VIDIOC_ENUMINPUT, vidioc_enum_input);
+ SET_VALID_IOCTL(ops, VIDIOC_G_INPUT, vidioc_g_input);
+ SET_VALID_IOCTL(ops, VIDIOC_S_INPUT, vidioc_s_input);
+ SET_VALID_IOCTL(ops, VIDIOC_G_PARM, vidioc_g_parm);
+ SET_VALID_IOCTL(ops, VIDIOC_S_PARM, vidioc_s_parm);
} else if (is_sdr && is_rx) {
/* SDR receiver specific ioctls */
SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_sdr_cap);
@@ -702,8 +715,8 @@ static void determine_valid_ioctls(struct video_device *vdev)
SET_VALID_IOCTL(ops, VIDIOC_STREAMOFF, vidioc_streamoff);
}
- if (is_vid || is_vbi || is_tch || is_meta) {
- /* ioctls valid for video, vbi, touch and metadata */
+ if (is_vid || is_vbi || is_meta) {
+ /* ioctls valid for video, vbi and metadata */
if (ops->vidioc_s_std)
set_bit(_IOC_NR(VIDIOC_ENUMSTD), valid_ioctls);
SET_VALID_IOCTL(ops, VIDIOC_S_STD, vidioc_s_std);
@@ -727,8 +740,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
SET_VALID_IOCTL(ops, VIDIOC_G_AUDOUT, vidioc_g_audout);
SET_VALID_IOCTL(ops, VIDIOC_S_AUDOUT, vidioc_s_audout);
}
- if (ops->vidioc_g_parm || (vdev->vfl_type == VFL_TYPE_GRABBER &&
- ops->vidioc_g_std))
+ if (ops->vidioc_g_parm || ops->vidioc_g_std)
set_bit(_IOC_NR(VIDIOC_G_PARM), valid_ioctls);
SET_VALID_IOCTL(ops, VIDIOC_S_PARM, vidioc_s_parm);
SET_VALID_IOCTL(ops, VIDIOC_S_DV_TIMINGS, vidioc_s_dv_timings);
--
2.23.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCHv6 4/3] v4l2-dev: disable frequency and tuner ioctls for touch
2019-10-14 8:40 ` [PATCHv6 3/3] v4l2-dev: fix is_tch checks Hans Verkuil
@ 2019-10-14 12:01 ` Hans Verkuil
2019-10-14 21:43 ` Laurent Pinchart
2019-10-14 21:42 ` [PATCHv6 3/3] v4l2-dev: fix is_tch checks Laurent Pinchart
1 sibling, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2019-10-14 12:01 UTC (permalink / raw)
To: linux-media; +Cc: Vandana BN, Sakari Ailus, Laurent Pinchart
Touch devices have obviously no tuner, so don't attempt to enable those ioctls
for such devices.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index cec588b04711..da42d172714a 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -581,8 +581,10 @@ static void determine_valid_ioctls(struct video_device *vdev)
set_bit(_IOC_NR(VIDIOC_TRY_EXT_CTRLS), valid_ioctls);
if (vdev->ctrl_handler || ops->vidioc_querymenu)
set_bit(_IOC_NR(VIDIOC_QUERYMENU), valid_ioctls);
- SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
- SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
+ if (!is_tch) {
+ SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
+ SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
+ }
SET_VALID_IOCTL(ops, VIDIOC_LOG_STATUS, vidioc_log_status);
#ifdef CONFIG_VIDEO_ADV_DEBUG
set_bit(_IOC_NR(VIDIOC_DBG_G_CHIP_INFO), valid_ioctls);
@@ -754,7 +756,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
SET_VALID_IOCTL(ops, VIDIOC_G_MODULATOR, vidioc_g_modulator);
SET_VALID_IOCTL(ops, VIDIOC_S_MODULATOR, vidioc_s_modulator);
}
- if (is_rx) {
+ if (is_rx && !is_tch) {
/* receiver only ioctls */
SET_VALID_IOCTL(ops, VIDIOC_G_TUNER, vidioc_g_tuner);
SET_VALID_IOCTL(ops, VIDIOC_S_TUNER, vidioc_s_tuner);
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCHv6 1/3] v4l2-core: correctly validate video and metadata ioctls
2019-10-14 8:40 ` [PATCHv6 1/3] v4l2-core: correctly validate video and metadata ioctls Hans Verkuil
@ 2019-10-14 21:36 ` Laurent Pinchart
2019-10-15 6:54 ` Hans Verkuil
0 siblings, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2019-10-14 21:36 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Vandana BN, Sakari Ailus
Hi Hans,
Tank you for the patch.
On Mon, Oct 14, 2019 at 10:40:19AM +0200, Hans Verkuil wrote:
> From: Vandana BN <bnvandana@gmail.com>
>
> If the type is VFL_TYPE_GRABBER, then also check device_caps
> to see if the video device supports video and/or metadata and
> disable unneeded ioctls.
>
> Without this change, format ioctls for both video and metadata devices
> could be called on both device nodes. This is true for other ioctls as
> well, even if the device supports only video or metadata.
>
> Metadata devices act similar to VBI devices w.r.t. which ioctls should
> be enabled. This makes sense since VBI *is* metadata.
>
> Signed-off-by: Vandana BN <bnvandana@gmail.com>
> Co-Developed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
> drivers/media/v4l2-core/v4l2-dev.c | 62 +++++++++++++++++-----------
> drivers/media/v4l2-core/v4l2-ioctl.c | 16 +++++--
> 2 files changed, 52 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 4037689a945a..1bf543932e4f 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -533,13 +533,23 @@ static int get_index(struct video_device *vdev)
> */
> static void determine_valid_ioctls(struct video_device *vdev)
> {
> + const u32 vid_caps = V4L2_CAP_VIDEO_CAPTURE |
> + V4L2_CAP_VIDEO_CAPTURE_MPLANE |
> + V4L2_CAP_VIDEO_OUTPUT |
> + V4L2_CAP_VIDEO_OUTPUT_MPLANE |
> + V4L2_CAP_VIDEO_M2M | V4L2_CAP_VIDEO_M2M_MPLANE;
> + const u32 meta_caps = V4L2_CAP_META_CAPTURE |
> + V4L2_CAP_META_OUTPUT;
> DECLARE_BITMAP(valid_ioctls, BASE_VIDIOC_PRIVATE);
> const struct v4l2_ioctl_ops *ops = vdev->ioctl_ops;
> - bool is_vid = vdev->vfl_type == VFL_TYPE_GRABBER;
> + bool is_vid = vdev->vfl_type == VFL_TYPE_GRABBER &&
> + (vdev->device_caps & vid_caps);
> bool is_vbi = vdev->vfl_type == VFL_TYPE_VBI;
> bool is_radio = vdev->vfl_type == VFL_TYPE_RADIO;
> bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
> bool is_tch = vdev->vfl_type == VFL_TYPE_TOUCH;
> + bool is_meta = vdev->vfl_type == VFL_TYPE_GRABBER &&
> + (vdev->device_caps & meta_caps);
> bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
> bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
>
> @@ -587,39 +597,31 @@ static void determine_valid_ioctls(struct video_device *vdev)
> set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
>
> if (is_vid || is_tch) {
> - /* video and metadata specific ioctls */
> + /* video and touch specific ioctls */
> if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
> - ops->vidioc_enum_fmt_vid_overlay ||
> - ops->vidioc_enum_fmt_meta_cap)) ||
> - (is_tx && (ops->vidioc_enum_fmt_vid_out ||
> - ops->vidioc_enum_fmt_meta_out)))
> + ops->vidioc_enum_fmt_vid_overlay)) ||
> + (is_tx && ops->vidioc_enum_fmt_vid_out))
> set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
> if ((is_rx && (ops->vidioc_g_fmt_vid_cap ||
> ops->vidioc_g_fmt_vid_cap_mplane ||
> - ops->vidioc_g_fmt_vid_overlay ||
> - ops->vidioc_g_fmt_meta_cap)) ||
> + ops->vidioc_g_fmt_vid_overlay)) ||
> (is_tx && (ops->vidioc_g_fmt_vid_out ||
> ops->vidioc_g_fmt_vid_out_mplane ||
> - ops->vidioc_g_fmt_vid_out_overlay ||
> - ops->vidioc_g_fmt_meta_out)))
> + ops->vidioc_g_fmt_vid_out_overlay)))
> set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
> if ((is_rx && (ops->vidioc_s_fmt_vid_cap ||
> ops->vidioc_s_fmt_vid_cap_mplane ||
> - ops->vidioc_s_fmt_vid_overlay ||
> - ops->vidioc_s_fmt_meta_cap)) ||
> + ops->vidioc_s_fmt_vid_overlay)) ||
> (is_tx && (ops->vidioc_s_fmt_vid_out ||
> ops->vidioc_s_fmt_vid_out_mplane ||
> - ops->vidioc_s_fmt_vid_out_overlay ||
> - ops->vidioc_s_fmt_meta_out)))
> + ops->vidioc_s_fmt_vid_out_overlay)))
> set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
> if ((is_rx && (ops->vidioc_try_fmt_vid_cap ||
> ops->vidioc_try_fmt_vid_cap_mplane ||
> - ops->vidioc_try_fmt_vid_overlay ||
> - ops->vidioc_try_fmt_meta_cap)) ||
> + ops->vidioc_try_fmt_vid_overlay)) ||
> (is_tx && (ops->vidioc_try_fmt_vid_out ||
> ops->vidioc_try_fmt_vid_out_mplane ||
> - ops->vidioc_try_fmt_vid_out_overlay ||
> - ops->vidioc_try_fmt_meta_out)))
> + ops->vidioc_try_fmt_vid_out_overlay)))
> set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
> SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
> SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
> @@ -641,7 +643,21 @@ static void determine_valid_ioctls(struct video_device *vdev)
> set_bit(_IOC_NR(VIDIOC_S_CROP), valid_ioctls);
> SET_VALID_IOCTL(ops, VIDIOC_G_SELECTION, vidioc_g_selection);
> SET_VALID_IOCTL(ops, VIDIOC_S_SELECTION, vidioc_s_selection);
> - } else if (is_vbi) {
> + }
Here you allow for is_vid and is_meta to be both true.
> + if (is_meta && is_rx) {
> + /* metadata capture specific ioctls */
> + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_meta_cap);
> + SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_meta_cap);
> + SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_meta_cap);
> + SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_meta_cap);
> + } else if (is_meta && is_tx) {
> + /* metadata output specific ioctls */
> + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_meta_out);
> + SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_meta_out);
> + SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_meta_out);
> + SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_meta_out);
> + }
And here for is_vbi to be true as well. But further down (not shown in
this patch), is_sdr is still considered to be mutually exclusive with
is_vbi. This is a bit confusing, even if I think it's correct.
> + if (is_vbi) {
> /* vbi specific ioctls */
> if ((is_rx && (ops->vidioc_g_fmt_vbi_cap ||
> ops->vidioc_g_fmt_sliced_vbi_cap)) ||
> @@ -681,8 +697,8 @@ static void determine_valid_ioctls(struct video_device *vdev)
> set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
> }
>
> - if (is_vid || is_vbi || is_sdr || is_tch) {
> - /* ioctls valid for video, metadata, vbi or sdr */
> + if (is_vid || is_vbi || is_sdr || is_tch || is_meta) {
> + /* ioctls valid for video, vbi, sdr, touch and metadata */
> SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
> SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
> SET_VALID_IOCTL(ops, VIDIOC_QBUF, vidioc_qbuf);
> @@ -694,8 +710,8 @@ static void determine_valid_ioctls(struct video_device *vdev)
> SET_VALID_IOCTL(ops, VIDIOC_STREAMOFF, vidioc_streamoff);
> }
>
> - if (is_vid || is_vbi || is_tch) {
> - /* ioctls valid for video or vbi */
> + if (is_vid || is_vbi || is_tch || is_meta) {
> + /* ioctls valid for video, vbi, touch and metadata */
Are all those ioctls valid for metadata ?
> if (ops->vidioc_s_std)
> set_bit(_IOC_NR(VIDIOC_ENUMSTD), valid_ioctls);
> SET_VALID_IOCTL(ops, VIDIOC_S_STD, vidioc_s_std);
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 51b912743f0f..20b3107dd4e8 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -932,12 +932,22 @@ static int check_ext_ctrls(struct v4l2_ext_controls *c, int allow_priv)
>
> static int check_fmt(struct file *file, enum v4l2_buf_type type)
> {
> + const u32 vid_caps = V4L2_CAP_VIDEO_CAPTURE |
> + V4L2_CAP_VIDEO_CAPTURE_MPLANE |
> + V4L2_CAP_VIDEO_OUTPUT |
> + V4L2_CAP_VIDEO_OUTPUT_MPLANE |
> + V4L2_CAP_VIDEO_M2M | V4L2_CAP_VIDEO_M2M_MPLANE;
> + const u32 meta_caps = V4L2_CAP_META_CAPTURE |
> + V4L2_CAP_META_OUTPUT;
> struct video_device *vfd = video_devdata(file);
> const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops;
> - bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
> + bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER &&
> + (vfd->device_caps & vid_caps);
> bool is_vbi = vfd->vfl_type == VFL_TYPE_VBI;
> bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
> bool is_tch = vfd->vfl_type == VFL_TYPE_TOUCH;
> + bool is_meta = vfd->vfl_type == VFL_TYPE_GRABBER &&
> + (vfd->device_caps & meta_caps);
> bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
> bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
>
> @@ -996,11 +1006,11 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
> return 0;
> break;
> case V4L2_BUF_TYPE_META_CAPTURE:
> - if (is_vid && is_rx && ops->vidioc_g_fmt_meta_cap)
> + if (is_meta && is_rx && ops->vidioc_g_fmt_meta_cap)
> return 0;
> break;
> case V4L2_BUF_TYPE_META_OUTPUT:
> - if (is_vid && is_tx && ops->vidioc_g_fmt_meta_out)
> + if (is_meta && is_tx && ops->vidioc_g_fmt_meta_out)
> return 0;
> break;
> default:
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv6 2/3] v4l2-dev: simplify the SDR checks
2019-10-14 8:40 ` [PATCHv6 2/3] v4l2-dev: simplify the SDR checks Hans Verkuil
@ 2019-10-14 21:39 ` Laurent Pinchart
0 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2019-10-14 21:39 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Vandana BN, Sakari Ailus
Hi Hans,
Thank you for the patch.
On Mon, Oct 14, 2019 at 10:40:20AM +0200, Hans Verkuil wrote:
> In determine_valid_ioctls() we can use SET_VALID_IOCTL to enable
> ioctls for SDR, simplifying the code.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/v4l2-core/v4l2-dev.c | 24 ++++++++----------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 1bf543932e4f..27fb96a6c2a8 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -677,24 +677,16 @@ static void determine_valid_ioctls(struct video_device *vdev)
> SET_VALID_IOCTL(ops, VIDIOC_G_SLICED_VBI_CAP, vidioc_g_sliced_vbi_cap);
> } else if (is_sdr && is_rx) {
> /* SDR receiver specific ioctls */
> - if (ops->vidioc_enum_fmt_sdr_cap)
> - set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
> - if (ops->vidioc_g_fmt_sdr_cap)
> - set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
> - if (ops->vidioc_s_fmt_sdr_cap)
> - set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
> - if (ops->vidioc_try_fmt_sdr_cap)
> - set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
> + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_sdr_cap);
> + SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_sdr_cap);
> + SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_sdr_cap);
> + SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_sdr_cap);
> } else if (is_sdr && is_tx) {
> /* SDR transmitter specific ioctls */
> - if (ops->vidioc_enum_fmt_sdr_out)
> - set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
> - if (ops->vidioc_g_fmt_sdr_out)
> - set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
> - if (ops->vidioc_s_fmt_sdr_out)
> - set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
> - if (ops->vidioc_try_fmt_sdr_out)
> - set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
> + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_sdr_out);
> + SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_sdr_out);
> + SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_sdr_out);
> + SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_sdr_out);
> }
>
> if (is_vid || is_vbi || is_sdr || is_tch || is_meta) {
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv6 3/3] v4l2-dev: fix is_tch checks
2019-10-14 8:40 ` [PATCHv6 3/3] v4l2-dev: fix is_tch checks Hans Verkuil
2019-10-14 12:01 ` [PATCHv6 4/3] v4l2-dev: disable frequency and tuner ioctls for touch Hans Verkuil
@ 2019-10-14 21:42 ` Laurent Pinchart
2019-10-15 6:44 ` Hans Verkuil
1 sibling, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2019-10-14 21:42 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Vandana BN, Sakari Ailus
Hi Hans,
Thank you for the patch.
On Mon, Oct 14, 2019 at 10:40:21AM +0200, Hans Verkuil wrote:
> Touch devices mark too many ioctls as valid. Restrict the list of
> valid ioctls for touch devices.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
> drivers/media/v4l2-core/v4l2-dev.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 27fb96a6c2a8..cec588b04711 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -596,8 +596,8 @@ static void determine_valid_ioctls(struct video_device *vdev)
> if (ops->vidioc_enum_freq_bands || ops->vidioc_g_tuner || ops->vidioc_g_modulator)
> set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
>
> - if (is_vid || is_tch) {
> - /* video and touch specific ioctls */
> + if (is_vid) {
> + /* video specific ioctls */
> if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
> ops->vidioc_enum_fmt_vid_overlay)) ||
> (is_tx && ops->vidioc_enum_fmt_vid_out))
> @@ -675,6 +675,19 @@ static void determine_valid_ioctls(struct video_device *vdev)
> ops->vidioc_try_fmt_sliced_vbi_out)))
> set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
> SET_VALID_IOCTL(ops, VIDIOC_G_SLICED_VBI_CAP, vidioc_g_sliced_vbi_cap);
> + } else if (is_tch) {
> + /* touch specific ioctls */
> + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_vid_cap);
> + SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_vid_cap);
> + SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_vid_cap);
> + SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_vid_cap);
> + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMESIZES, vidioc_enum_framesizes);
> + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMEINTERVALS, vidioc_enum_frameintervals);
> + SET_VALID_IOCTL(ops, VIDIOC_ENUMINPUT, vidioc_enum_input);
> + SET_VALID_IOCTL(ops, VIDIOC_G_INPUT, vidioc_g_input);
> + SET_VALID_IOCTL(ops, VIDIOC_S_INPUT, vidioc_s_input);
> + SET_VALID_IOCTL(ops, VIDIOC_G_PARM, vidioc_g_parm);
> + SET_VALID_IOCTL(ops, VIDIOC_S_PARM, vidioc_s_parm);
> } else if (is_sdr && is_rx) {
> /* SDR receiver specific ioctls */
> SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_sdr_cap);
> @@ -702,8 +715,8 @@ static void determine_valid_ioctls(struct video_device *vdev)
> SET_VALID_IOCTL(ops, VIDIOC_STREAMOFF, vidioc_streamoff);
> }
>
> - if (is_vid || is_vbi || is_tch || is_meta) {
> - /* ioctls valid for video, vbi, touch and metadata */
> + if (is_vid || is_vbi || is_meta) {
> + /* ioctls valid for video, vbi and metadata */
> if (ops->vidioc_s_std)
> set_bit(_IOC_NR(VIDIOC_ENUMSTD), valid_ioctls);
> SET_VALID_IOCTL(ops, VIDIOC_S_STD, vidioc_s_std);
> @@ -727,8 +740,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
> SET_VALID_IOCTL(ops, VIDIOC_G_AUDOUT, vidioc_g_audout);
> SET_VALID_IOCTL(ops, VIDIOC_S_AUDOUT, vidioc_s_audout);
> }
> - if (ops->vidioc_g_parm || (vdev->vfl_type == VFL_TYPE_GRABBER &&
> - ops->vidioc_g_std))
> + if (ops->vidioc_g_parm || ops->vidioc_g_std)
> set_bit(_IOC_NR(VIDIOC_G_PARM), valid_ioctls);
This will become potentially valid for VBI devices, is it intended ?
> SET_VALID_IOCTL(ops, VIDIOC_S_PARM, vidioc_s_parm);
> SET_VALID_IOCTL(ops, VIDIOC_S_DV_TIMINGS, vidioc_s_dv_timings);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv6 4/3] v4l2-dev: disable frequency and tuner ioctls for touch
2019-10-14 12:01 ` [PATCHv6 4/3] v4l2-dev: disable frequency and tuner ioctls for touch Hans Verkuil
@ 2019-10-14 21:43 ` Laurent Pinchart
2019-10-15 6:47 ` Hans Verkuil
0 siblings, 1 reply; 13+ messages in thread
From: Laurent Pinchart @ 2019-10-14 21:43 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Vandana BN, Sakari Ailus
Hi Hans,
Thank you for the patch.
On Mon, Oct 14, 2019 at 02:01:05PM +0200, Hans Verkuil wrote:
> Touch devices have obviously no tuner, so don't attempt to enable those ioctls
> for such devices.
Shouldn't this be disabled for pure metadata devices (is_meta &&
!is_vid) too ?
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index cec588b04711..da42d172714a 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -581,8 +581,10 @@ static void determine_valid_ioctls(struct video_device *vdev)
> set_bit(_IOC_NR(VIDIOC_TRY_EXT_CTRLS), valid_ioctls);
> if (vdev->ctrl_handler || ops->vidioc_querymenu)
> set_bit(_IOC_NR(VIDIOC_QUERYMENU), valid_ioctls);
> - SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
> - SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
> + if (!is_tch) {
> + SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
> + SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
> + }
> SET_VALID_IOCTL(ops, VIDIOC_LOG_STATUS, vidioc_log_status);
> #ifdef CONFIG_VIDEO_ADV_DEBUG
> set_bit(_IOC_NR(VIDIOC_DBG_G_CHIP_INFO), valid_ioctls);
> @@ -754,7 +756,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
> SET_VALID_IOCTL(ops, VIDIOC_G_MODULATOR, vidioc_g_modulator);
> SET_VALID_IOCTL(ops, VIDIOC_S_MODULATOR, vidioc_s_modulator);
> }
> - if (is_rx) {
> + if (is_rx && !is_tch) {
> /* receiver only ioctls */
> SET_VALID_IOCTL(ops, VIDIOC_G_TUNER, vidioc_g_tuner);
> SET_VALID_IOCTL(ops, VIDIOC_S_TUNER, vidioc_s_tuner);
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv6 3/3] v4l2-dev: fix is_tch checks
2019-10-14 21:42 ` [PATCHv6 3/3] v4l2-dev: fix is_tch checks Laurent Pinchart
@ 2019-10-15 6:44 ` Hans Verkuil
0 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-10-15 6:44 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, Vandana BN, Sakari Ailus
On 10/14/19 11:42 PM, Laurent Pinchart wrote:
> Hi Hans,
>
> Thank you for the patch.
>
> On Mon, Oct 14, 2019 at 10:40:21AM +0200, Hans Verkuil wrote:
>> Touch devices mark too many ioctls as valid. Restrict the list of
>> valid ioctls for touch devices.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>> drivers/media/v4l2-core/v4l2-dev.c | 24 ++++++++++++++++++------
>> 1 file changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
>> index 27fb96a6c2a8..cec588b04711 100644
>> --- a/drivers/media/v4l2-core/v4l2-dev.c
>> +++ b/drivers/media/v4l2-core/v4l2-dev.c
>> @@ -596,8 +596,8 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> if (ops->vidioc_enum_freq_bands || ops->vidioc_g_tuner || ops->vidioc_g_modulator)
>> set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
>>
>> - if (is_vid || is_tch) {
>> - /* video and touch specific ioctls */
>> + if (is_vid) {
>> + /* video specific ioctls */
>> if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
>> ops->vidioc_enum_fmt_vid_overlay)) ||
>> (is_tx && ops->vidioc_enum_fmt_vid_out))
>> @@ -675,6 +675,19 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> ops->vidioc_try_fmt_sliced_vbi_out)))
>> set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
>> SET_VALID_IOCTL(ops, VIDIOC_G_SLICED_VBI_CAP, vidioc_g_sliced_vbi_cap);
>> + } else if (is_tch) {
>> + /* touch specific ioctls */
>> + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_vid_cap);
>> + SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_vid_cap);
>> + SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_vid_cap);
>> + SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_vid_cap);
>> + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMESIZES, vidioc_enum_framesizes);
>> + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMEINTERVALS, vidioc_enum_frameintervals);
>> + SET_VALID_IOCTL(ops, VIDIOC_ENUMINPUT, vidioc_enum_input);
>> + SET_VALID_IOCTL(ops, VIDIOC_G_INPUT, vidioc_g_input);
>> + SET_VALID_IOCTL(ops, VIDIOC_S_INPUT, vidioc_s_input);
>> + SET_VALID_IOCTL(ops, VIDIOC_G_PARM, vidioc_g_parm);
>> + SET_VALID_IOCTL(ops, VIDIOC_S_PARM, vidioc_s_parm);
>> } else if (is_sdr && is_rx) {
>> /* SDR receiver specific ioctls */
>> SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_sdr_cap);
>> @@ -702,8 +715,8 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> SET_VALID_IOCTL(ops, VIDIOC_STREAMOFF, vidioc_streamoff);
>> }
>>
>> - if (is_vid || is_vbi || is_tch || is_meta) {
>> - /* ioctls valid for video, vbi, touch and metadata */
>> + if (is_vid || is_vbi || is_meta) {
>> + /* ioctls valid for video, vbi and metadata */
>> if (ops->vidioc_s_std)
>> set_bit(_IOC_NR(VIDIOC_ENUMSTD), valid_ioctls);
>> SET_VALID_IOCTL(ops, VIDIOC_S_STD, vidioc_s_std);
>> @@ -727,8 +740,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> SET_VALID_IOCTL(ops, VIDIOC_G_AUDOUT, vidioc_g_audout);
>> SET_VALID_IOCTL(ops, VIDIOC_S_AUDOUT, vidioc_s_audout);
>> }
>> - if (ops->vidioc_g_parm || (vdev->vfl_type == VFL_TYPE_GRABBER &&
>> - ops->vidioc_g_std))
>> + if (ops->vidioc_g_parm || ops->vidioc_g_std)
>> set_bit(_IOC_NR(VIDIOC_G_PARM), valid_ioctls);
>
> This will become potentially valid for VBI devices, is it intended ?
Actually, it wasn't intended, but it is correct :-)
VBI devices can have G_STD, and therefor G_PARM.
>
>> SET_VALID_IOCTL(ops, VIDIOC_S_PARM, vidioc_s_parm);
>> SET_VALID_IOCTL(ops, VIDIOC_S_DV_TIMINGS, vidioc_s_dv_timings);
>
Regards,
Hans
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv6 4/3] v4l2-dev: disable frequency and tuner ioctls for touch
2019-10-14 21:43 ` Laurent Pinchart
@ 2019-10-15 6:47 ` Hans Verkuil
0 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-10-15 6:47 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, Vandana BN, Sakari Ailus
On 10/14/19 11:43 PM, Laurent Pinchart wrote:
> Hi Hans,
>
> Thank you for the patch.
>
> On Mon, Oct 14, 2019 at 02:01:05PM +0200, Hans Verkuil wrote:
>> Touch devices have obviously no tuner, so don't attempt to enable those ioctls
>> for such devices.
>
> Shouldn't this be disabled for pure metadata devices (is_meta &&
> !is_vid) too ?
No. It doesn't matter for this whether metadata is a separate or combined device,
in both cases a meta device can be connected to a source that is controlled via
a tuner.
Regards,
Hans
>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
>> index cec588b04711..da42d172714a 100644
>> --- a/drivers/media/v4l2-core/v4l2-dev.c
>> +++ b/drivers/media/v4l2-core/v4l2-dev.c
>> @@ -581,8 +581,10 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> set_bit(_IOC_NR(VIDIOC_TRY_EXT_CTRLS), valid_ioctls);
>> if (vdev->ctrl_handler || ops->vidioc_querymenu)
>> set_bit(_IOC_NR(VIDIOC_QUERYMENU), valid_ioctls);
>> - SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
>> - SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
>> + if (!is_tch) {
>> + SET_VALID_IOCTL(ops, VIDIOC_G_FREQUENCY, vidioc_g_frequency);
>> + SET_VALID_IOCTL(ops, VIDIOC_S_FREQUENCY, vidioc_s_frequency);
>> + }
>> SET_VALID_IOCTL(ops, VIDIOC_LOG_STATUS, vidioc_log_status);
>> #ifdef CONFIG_VIDEO_ADV_DEBUG
>> set_bit(_IOC_NR(VIDIOC_DBG_G_CHIP_INFO), valid_ioctls);
>> @@ -754,7 +756,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> SET_VALID_IOCTL(ops, VIDIOC_G_MODULATOR, vidioc_g_modulator);
>> SET_VALID_IOCTL(ops, VIDIOC_S_MODULATOR, vidioc_s_modulator);
>> }
>> - if (is_rx) {
>> + if (is_rx && !is_tch) {
>> /* receiver only ioctls */
>> SET_VALID_IOCTL(ops, VIDIOC_G_TUNER, vidioc_g_tuner);
>> SET_VALID_IOCTL(ops, VIDIOC_S_TUNER, vidioc_s_tuner);
>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv6 1/3] v4l2-core: correctly validate video and metadata ioctls
2019-10-14 21:36 ` Laurent Pinchart
@ 2019-10-15 6:54 ` Hans Verkuil
0 siblings, 0 replies; 13+ messages in thread
From: Hans Verkuil @ 2019-10-15 6:54 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, Vandana BN, Sakari Ailus
On 10/14/19 11:36 PM, Laurent Pinchart wrote:
> Hi Hans,
>
> Tank you for the patch.
>
> On Mon, Oct 14, 2019 at 10:40:19AM +0200, Hans Verkuil wrote:
>> From: Vandana BN <bnvandana@gmail.com>
>>
>> If the type is VFL_TYPE_GRABBER, then also check device_caps
>> to see if the video device supports video and/or metadata and
>> disable unneeded ioctls.
>>
>> Without this change, format ioctls for both video and metadata devices
>> could be called on both device nodes. This is true for other ioctls as
>> well, even if the device supports only video or metadata.
>>
>> Metadata devices act similar to VBI devices w.r.t. which ioctls should
>> be enabled. This makes sense since VBI *is* metadata.
>>
>> Signed-off-by: Vandana BN <bnvandana@gmail.com>
>> Co-Developed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>> drivers/media/v4l2-core/v4l2-dev.c | 62 +++++++++++++++++-----------
>> drivers/media/v4l2-core/v4l2-ioctl.c | 16 +++++--
>> 2 files changed, 52 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
>> index 4037689a945a..1bf543932e4f 100644
>> --- a/drivers/media/v4l2-core/v4l2-dev.c
>> +++ b/drivers/media/v4l2-core/v4l2-dev.c
>> @@ -533,13 +533,23 @@ static int get_index(struct video_device *vdev)
>> */
>> static void determine_valid_ioctls(struct video_device *vdev)
>> {
>> + const u32 vid_caps = V4L2_CAP_VIDEO_CAPTURE |
>> + V4L2_CAP_VIDEO_CAPTURE_MPLANE |
>> + V4L2_CAP_VIDEO_OUTPUT |
>> + V4L2_CAP_VIDEO_OUTPUT_MPLANE |
>> + V4L2_CAP_VIDEO_M2M | V4L2_CAP_VIDEO_M2M_MPLANE;
>> + const u32 meta_caps = V4L2_CAP_META_CAPTURE |
>> + V4L2_CAP_META_OUTPUT;
>> DECLARE_BITMAP(valid_ioctls, BASE_VIDIOC_PRIVATE);
>> const struct v4l2_ioctl_ops *ops = vdev->ioctl_ops;
>> - bool is_vid = vdev->vfl_type == VFL_TYPE_GRABBER;
>> + bool is_vid = vdev->vfl_type == VFL_TYPE_GRABBER &&
>> + (vdev->device_caps & vid_caps);
>> bool is_vbi = vdev->vfl_type == VFL_TYPE_VBI;
>> bool is_radio = vdev->vfl_type == VFL_TYPE_RADIO;
>> bool is_sdr = vdev->vfl_type == VFL_TYPE_SDR;
>> bool is_tch = vdev->vfl_type == VFL_TYPE_TOUCH;
>> + bool is_meta = vdev->vfl_type == VFL_TYPE_GRABBER &&
>> + (vdev->device_caps & meta_caps);
>> bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
>> bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
>>
>> @@ -587,39 +597,31 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> set_bit(_IOC_NR(VIDIOC_ENUM_FREQ_BANDS), valid_ioctls);
>>
>> if (is_vid || is_tch) {
>> - /* video and metadata specific ioctls */
>> + /* video and touch specific ioctls */
>> if ((is_rx && (ops->vidioc_enum_fmt_vid_cap ||
>> - ops->vidioc_enum_fmt_vid_overlay ||
>> - ops->vidioc_enum_fmt_meta_cap)) ||
>> - (is_tx && (ops->vidioc_enum_fmt_vid_out ||
>> - ops->vidioc_enum_fmt_meta_out)))
>> + ops->vidioc_enum_fmt_vid_overlay)) ||
>> + (is_tx && ops->vidioc_enum_fmt_vid_out))
>> set_bit(_IOC_NR(VIDIOC_ENUM_FMT), valid_ioctls);
>> if ((is_rx && (ops->vidioc_g_fmt_vid_cap ||
>> ops->vidioc_g_fmt_vid_cap_mplane ||
>> - ops->vidioc_g_fmt_vid_overlay ||
>> - ops->vidioc_g_fmt_meta_cap)) ||
>> + ops->vidioc_g_fmt_vid_overlay)) ||
>> (is_tx && (ops->vidioc_g_fmt_vid_out ||
>> ops->vidioc_g_fmt_vid_out_mplane ||
>> - ops->vidioc_g_fmt_vid_out_overlay ||
>> - ops->vidioc_g_fmt_meta_out)))
>> + ops->vidioc_g_fmt_vid_out_overlay)))
>> set_bit(_IOC_NR(VIDIOC_G_FMT), valid_ioctls);
>> if ((is_rx && (ops->vidioc_s_fmt_vid_cap ||
>> ops->vidioc_s_fmt_vid_cap_mplane ||
>> - ops->vidioc_s_fmt_vid_overlay ||
>> - ops->vidioc_s_fmt_meta_cap)) ||
>> + ops->vidioc_s_fmt_vid_overlay)) ||
>> (is_tx && (ops->vidioc_s_fmt_vid_out ||
>> ops->vidioc_s_fmt_vid_out_mplane ||
>> - ops->vidioc_s_fmt_vid_out_overlay ||
>> - ops->vidioc_s_fmt_meta_out)))
>> + ops->vidioc_s_fmt_vid_out_overlay)))
>> set_bit(_IOC_NR(VIDIOC_S_FMT), valid_ioctls);
>> if ((is_rx && (ops->vidioc_try_fmt_vid_cap ||
>> ops->vidioc_try_fmt_vid_cap_mplane ||
>> - ops->vidioc_try_fmt_vid_overlay ||
>> - ops->vidioc_try_fmt_meta_cap)) ||
>> + ops->vidioc_try_fmt_vid_overlay)) ||
>> (is_tx && (ops->vidioc_try_fmt_vid_out ||
>> ops->vidioc_try_fmt_vid_out_mplane ||
>> - ops->vidioc_try_fmt_vid_out_overlay ||
>> - ops->vidioc_try_fmt_meta_out)))
>> + ops->vidioc_try_fmt_vid_out_overlay)))
>> set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
>> SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
>> SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
>> @@ -641,7 +643,21 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> set_bit(_IOC_NR(VIDIOC_S_CROP), valid_ioctls);
>> SET_VALID_IOCTL(ops, VIDIOC_G_SELECTION, vidioc_g_selection);
>> SET_VALID_IOCTL(ops, VIDIOC_S_SELECTION, vidioc_s_selection);
>> - } else if (is_vbi) {
>> + }
>
> Here you allow for is_vid and is_meta to be both true.
>
>> + if (is_meta && is_rx) {
>> + /* metadata capture specific ioctls */
>> + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_meta_cap);
>> + SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_meta_cap);
>> + SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_meta_cap);
>> + SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_meta_cap);
>> + } else if (is_meta && is_tx) {
>> + /* metadata output specific ioctls */
>> + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_meta_out);
>> + SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_meta_out);
>> + SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_meta_out);
>> + SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_meta_out);
>> + }
>
> And here for is_vbi to be true as well. But further down (not shown in
> this patch), is_sdr is still considered to be mutually exclusive with
> is_vbi. This is a bit confusing, even if I think it's correct.
After patch 3 it looks a bit better. The code is correct, so I don't want to
change it.
>
>> + if (is_vbi) {
>> /* vbi specific ioctls */
>> if ((is_rx && (ops->vidioc_g_fmt_vbi_cap ||
>> ops->vidioc_g_fmt_sliced_vbi_cap)) ||
>> @@ -681,8 +697,8 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> set_bit(_IOC_NR(VIDIOC_TRY_FMT), valid_ioctls);
>> }
>>
>> - if (is_vid || is_vbi || is_sdr || is_tch) {
>> - /* ioctls valid for video, metadata, vbi or sdr */
>> + if (is_vid || is_vbi || is_sdr || is_tch || is_meta) {
>> + /* ioctls valid for video, vbi, sdr, touch and metadata */
>> SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
>> SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
>> SET_VALID_IOCTL(ops, VIDIOC_QBUF, vidioc_qbuf);
>> @@ -694,8 +710,8 @@ static void determine_valid_ioctls(struct video_device *vdev)
>> SET_VALID_IOCTL(ops, VIDIOC_STREAMOFF, vidioc_streamoff);
>> }
>>
>> - if (is_vid || is_vbi || is_tch) {
>> - /* ioctls valid for video or vbi */
>> + if (is_vid || is_vbi || is_tch || is_meta) {
>> + /* ioctls valid for video, vbi, touch and metadata */
>
> Are all those ioctls valid for metadata ?
Yes, when metadata is used with SDTV/HDTV receivers, potentially with tuners.
It should behave the same as a VBI device, which is also metadata.
Regards,
Hans
>
>> if (ops->vidioc_s_std)
>> set_bit(_IOC_NR(VIDIOC_ENUMSTD), valid_ioctls);
>> SET_VALID_IOCTL(ops, VIDIOC_S_STD, vidioc_s_std);
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 51b912743f0f..20b3107dd4e8 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -932,12 +932,22 @@ static int check_ext_ctrls(struct v4l2_ext_controls *c, int allow_priv)
>>
>> static int check_fmt(struct file *file, enum v4l2_buf_type type)
>> {
>> + const u32 vid_caps = V4L2_CAP_VIDEO_CAPTURE |
>> + V4L2_CAP_VIDEO_CAPTURE_MPLANE |
>> + V4L2_CAP_VIDEO_OUTPUT |
>> + V4L2_CAP_VIDEO_OUTPUT_MPLANE |
>> + V4L2_CAP_VIDEO_M2M | V4L2_CAP_VIDEO_M2M_MPLANE;
>> + const u32 meta_caps = V4L2_CAP_META_CAPTURE |
>> + V4L2_CAP_META_OUTPUT;
>> struct video_device *vfd = video_devdata(file);
>> const struct v4l2_ioctl_ops *ops = vfd->ioctl_ops;
>> - bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER;
>> + bool is_vid = vfd->vfl_type == VFL_TYPE_GRABBER &&
>> + (vfd->device_caps & vid_caps);
>> bool is_vbi = vfd->vfl_type == VFL_TYPE_VBI;
>> bool is_sdr = vfd->vfl_type == VFL_TYPE_SDR;
>> bool is_tch = vfd->vfl_type == VFL_TYPE_TOUCH;
>> + bool is_meta = vfd->vfl_type == VFL_TYPE_GRABBER &&
>> + (vfd->device_caps & meta_caps);
>> bool is_rx = vfd->vfl_dir != VFL_DIR_TX;
>> bool is_tx = vfd->vfl_dir != VFL_DIR_RX;
>>
>> @@ -996,11 +1006,11 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
>> return 0;
>> break;
>> case V4L2_BUF_TYPE_META_CAPTURE:
>> - if (is_vid && is_rx && ops->vidioc_g_fmt_meta_cap)
>> + if (is_meta && is_rx && ops->vidioc_g_fmt_meta_cap)
>> return 0;
>> break;
>> case V4L2_BUF_TYPE_META_OUTPUT:
>> - if (is_vid && is_tx && ops->vidioc_g_fmt_meta_out)
>> + if (is_meta && is_tx && ops->vidioc_g_fmt_meta_out)
>> return 0;
>> break;
>> default:
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCHv6 0/3] v4l2-core: improve ioctl validation
2019-10-14 8:40 [PATCHv6 0/3] v4l2-core: improve ioctl validation Hans Verkuil
` (2 preceding siblings ...)
2019-10-14 8:40 ` [PATCHv6 3/3] v4l2-dev: fix is_tch checks Hans Verkuil
@ 2019-10-15 20:19 ` Laurent Pinchart
3 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2019-10-15 20:19 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, Vandana BN, Sakari Ailus
On Mon, Oct 14, 2019 at 10:40:18AM +0200, Hans Verkuil wrote:
> This supersedes https://www.mail-archive.com/linux-media@vger.kernel.org/msg150027.html
> based on feedback from Laurent:
>
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg150117.html
>
> and Sakari:
>
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg150129.html
>
> This v6 only moves some code from patch 1 to patch 3, the final code
> is the same as for v5. I plan to make a PR for this very soon together
> with the vivid metadata patches that need this.
>
> Regards,
>
> Hans
>
> Changes in v6:
>
> Patch 1/3 dropped the check against GRABBER for the g_parm ioctl,
> but that is too early: this should be done in patch 3/3 where this
> code no longer applies to touch devices (which was the reason for
> the GRABBER test).
>
> Changes in v5:
>
> I now check if a GRABBER device is a video or metadata device
> (or both!) by checking device_caps.
>
>
> Hans Verkuil (2):
> v4l2-dev: simplify the SDR checks
> v4l2-dev: fix is_tch checks
>
> Vandana BN (1):
> v4l2-core: correctly validate video and metadata ioctls
For the whole series,
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> drivers/media/v4l2-core/v4l2-dev.c | 104 ++++++++++++++++-----------
> drivers/media/v4l2-core/v4l2-ioctl.c | 16 ++++-
> 2 files changed, 75 insertions(+), 45 deletions(-)
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-10-15 20:19 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-14 8:40 [PATCHv6 0/3] v4l2-core: improve ioctl validation Hans Verkuil
2019-10-14 8:40 ` [PATCHv6 1/3] v4l2-core: correctly validate video and metadata ioctls Hans Verkuil
2019-10-14 21:36 ` Laurent Pinchart
2019-10-15 6:54 ` Hans Verkuil
2019-10-14 8:40 ` [PATCHv6 2/3] v4l2-dev: simplify the SDR checks Hans Verkuil
2019-10-14 21:39 ` Laurent Pinchart
2019-10-14 8:40 ` [PATCHv6 3/3] v4l2-dev: fix is_tch checks Hans Verkuil
2019-10-14 12:01 ` [PATCHv6 4/3] v4l2-dev: disable frequency and tuner ioctls for touch Hans Verkuil
2019-10-14 21:43 ` Laurent Pinchart
2019-10-15 6:47 ` Hans Verkuil
2019-10-14 21:42 ` [PATCHv6 3/3] v4l2-dev: fix is_tch checks Laurent Pinchart
2019-10-15 6:44 ` Hans Verkuil
2019-10-15 20:19 ` [PATCHv6 0/3] v4l2-core: improve ioctl validation Laurent Pinchart
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).