| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20220920-resend-v4l2-compliance-v3-1-598d33a15815@chromium.org>
Date: Tue, 03 Jan 2023 15:36:19 +0100
From: Ricardo Ribalda <ribalda@...omium.org>
To: Mauro Carvalho Chehab <mchehab@...nel.org>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Hans Verkuil <hverkuil@...all.nl>, linux-media@...r.kernel.org,
linux-kernel@...r.kernel.org,
Ricardo Ribalda <ribalda@...omium.org>,
Hans Verkuil <hans.verkuil@...co.com>,
Hans Verkuil <hverkuil-cisco@...all.nl>
Subject: [PATCH v3 1/8] media: uvcvideo: Check for INACTIVE in
uvc_ctrl_is_accessible()
From: Hans Verkuil <hverkuil@...all.nl>
Check for inactive controls in uvc_ctrl_is_accessible().
Use the new value for the master_id controls if present, otherwise
use the existing value to determine if it is OK to set the control.
Doing this here avoids attempting to set an inactive control, which
will return an error from the USB device, which returns an invalid
errorcode.
This fixes:
warn: v4l2-test-controls.cpp(483): s_ctrl returned EIO
warn: v4l2-test-controls.cpp(483): s_ctrl returned EIO
test VIDIOC_G/S_CTRL: OK
warn: v4l2-test-controls.cpp(739): s_ext_ctrls returned EIO
warn: v4l2-test-controls.cpp(739): s_ext_ctrls returned EIO
warn: v4l2-test-controls.cpp(816): s_ext_ctrls returned EIO
test VIDIOC_G/S/TRY_EXT_CTRLS: OK
Tested with:
v4l2-ctl -c auto_exposure=1
OK
v4l2-ctl -c exposure_time_absolute=251
OK
v4l2-ctl -c auto_exposure=3
OK
v4l2-ctl -c exposure_time_absolute=251
VIDIOC_S_EXT_CTRLS: failed: Input/output error
exposure_time_absolute: Input/output error
ERROR
v4l2-ctl -c auto_exposure=3,exposure_time_absolute=251,auto_exposure=1
v4l2-ctl -C auto_exposure,exposure_time_absolute
auto_exposure: 1
exposure_time_absolute: 251
Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Reviewed-by: Ricardo Ribalda <ribalda@...omium.org>
Signed-off-by: Hans Verkuil <hans.verkuil@...co.com>
---
drivers/media/usb/uvc/uvc_ctrl.c | 42 +++++++++++++++++++++++++++++++++++++++-
drivers/media/usb/uvc/uvc_v4l2.c | 3 +--
drivers/media/usb/uvc/uvcvideo.h | 3 ++-
3 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index c95a2229f4fa..6165d6b8e855 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1085,11 +1085,28 @@ static int uvc_query_v4l2_class(struct uvc_video_chain *chain, u32 req_id,
return 0;
}
+/*
+ * Check if control @v4l2_id can be accessed by the given control @ioctl
+ * (VIDIOC_G_EXT_CTRLS, VIDIOC_TRY_EXT_CTRLS or VIDIOC_S_EXT_CTRLS).
+ *
+ * For set operations on slave controls, check if the master's value is set to
+ * manual, either in the others controls set in the same ioctl call, or from
+ * the master's current value. This catches VIDIOC_S_EXT_CTRLS calls that
+ * set both the master and slave control, such as for instance setting
+ * auto_exposure=1, exposure_time_absolute=251.
+ */
int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
- bool read)
+ const struct v4l2_ext_controls *ctrls,
+ unsigned long ioctl)
{
+ struct uvc_control_mapping *master_map = NULL;
+ struct uvc_control *master_ctrl = NULL;
struct uvc_control_mapping *mapping;
struct uvc_control *ctrl;
+ bool read = ioctl == VIDIOC_G_EXT_CTRLS;
+ s32 val;
+ int ret;
+ int i;
if (__uvc_query_v4l2_class(chain, v4l2_id, 0) >= 0)
return -EACCES;
@@ -1104,6 +1121,29 @@ int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
if (!(ctrl->info.flags & UVC_CTRL_FLAG_SET_CUR) && !read)
return -EACCES;
+ if (ioctl != VIDIOC_S_EXT_CTRLS || !mapping->master_id)
+ return 0;
+
+ /*
+ * Iterate backwards in cases where the master control is accessed
+ * multiple times in the same ioctl. We want the last value.
+ */
+ for (i = ctrls->count - 1; i >= 0; i--) {
+ if (ctrls->controls[i].id == mapping->master_id)
+ return ctrls->controls[i].value ==
+ mapping->master_manual ? 0 : -EACCES;
+ }
+
+ __uvc_find_control(ctrl->entity, mapping->master_id, &master_map,
+ &master_ctrl, 0);
+
+ if (!master_ctrl || !(master_ctrl->info.flags & UVC_CTRL_FLAG_GET_CUR))
+ return 0;
+
+ ret = __uvc_ctrl_get(chain, master_ctrl, master_map, &val);
+ if (ret >= 0 && val != mapping->master_manual)
+ return -EACCES;
+
return 0;
}
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index f4d4c33b6dfb..3edb54c086b2 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1020,8 +1020,7 @@ static int uvc_ctrl_check_access(struct uvc_video_chain *chain,
int ret = 0;
for (i = 0; i < ctrls->count; ++ctrl, ++i) {
- ret = uvc_ctrl_is_accessible(chain, ctrl->id,
- ioctl == VIDIOC_G_EXT_CTRLS);
+ ret = uvc_ctrl_is_accessible(chain, ctrl->id, ctrls, ioctl);
if (ret)
break;
}
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index df93db259312..a151f583cd15 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -761,7 +761,8 @@ static inline int uvc_ctrl_rollback(struct uvc_fh *handle)
int uvc_ctrl_get(struct uvc_video_chain *chain, struct v4l2_ext_control *xctrl);
int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl);
int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
- bool read);
+ const struct v4l2_ext_controls *ctrls,
+ unsigned long ioctl);
int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
struct uvc_xu_control_query *xqry);
--
2.39.0.314.g84b9a713c41-goog-b4-0.11.0-dev-696ae
Powered by blists - more mailing lists