We can figure out if reading/writing a set of controls can fail without
accessing them by checking their flags.
This way we can honor the API closer:
If an error is found when validating the list of controls passed with
VIDIOC_G_EXT_CTRLS, then error_idx shall be set to ctrls->count to
indicate to userspace that no actual hardware was touched.
Fixes v4l2-compliance:
Control ioctls (Input 0):
warn: v4l2-test-controls.cpp(765): g_ext_ctrls(0) invalid
error_idx 0
fail: v4l2-test-controls.cpp(645): invalid error index write
only control
test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL
Signed-off-by: Ricardo Ribalda <riba...@chromium.org>
---
drivers/media/usb/uvc/uvc_v4l2.c | 69 +++++++++++++++++++++++---------
1 file changed, 51 insertions(+), 18 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 157310c0ca87..e956d833ed84 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1040,31 +1040,54 @@ static int uvc_ioctl_s_ctrl(struct file *file, void *fh,
return 0;
}
-static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
- struct v4l2_ext_controls *ctrls)
+static int uvc_ctrl_check_access(struct uvc_video_chain *chain,
+ struct v4l2_ext_controls *ctrls, bool read)
{
- struct uvc_fh *handle = fh;
- struct uvc_video_chain *chain = handle->chain;
struct v4l2_ext_control *ctrl = ctrls->controls;
unsigned int i;
- int ret;
+ int ret = 0;
- if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
- for (i = 0; i < ctrls->count; ++ctrl, ++i) {
- struct v4l2_queryctrl qc = { .id = ctrl->id };
+ for (i = 0; i < ctrls->count; ++ctrl, ++i) {
+ struct v4l2_queryctrl qc = { .id = ctrl->id };
- ret = uvc_query_v4l2_ctrl(chain, &qc);
- if (ret < 0) {
- ctrls->error_idx = i;
- return ret;
- }
+ ret = uvc_query_v4l2_ctrl(chain, &qc);
+ if (ret < 0)
+ break;
+ if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL) {
ctrl->value = qc.default_value;
+ continue;
}
- return 0;
+ if (qc.flags & V4L2_CTRL_FLAG_WRITE_ONLY && read) {
+ ret = -EACCES;
+ break;
+ }
+
+ if (qc.flags & V4L2_CTRL_FLAG_READ_ONLY && !read) {
+ ret = -EACCES;
+ break;
+ }
}
+ ctrls->error_idx = ctrls->count;
+
+ return ret;
+}
+
+static int uvc_ioctl_g_ext_ctrls(struct file *file, void *fh,
+ struct v4l2_ext_controls *ctrls)
+{
+ struct uvc_fh *handle = fh;
+ struct uvc_video_chain *chain = handle->chain;
+ struct v4l2_ext_control *ctrl = ctrls->controls;
+ unsigned int i;
+ int ret;
+
+ ret = uvc_ctrl_check_access(chain, ctrls, true);
+ if (ret < 0 || ctrls->which == V4L2_CTRL_WHICH_DEF_VAL)
+ return ret;
+
ret = uvc_ctrl_begin(chain);
if (ret < 0)
return ret;
@@ -1092,10 +1115,6 @@ static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh
*handle,
unsigned int i;
int ret;
- /* Default value cannot be changed */
- if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL)
- return -EINVAL;
-
ret = uvc_ctrl_begin(chain);
if (ret < 0)
return ret;
@@ -1121,6 +1140,16 @@ static int uvc_ioctl_s_ext_ctrls(struct file *file, void
*fh,
struct v4l2_ext_controls *ctrls)
{
struct uvc_fh *handle = fh;
+ struct uvc_video_chain *chain = handle->chain;
+ int ret;
+
+ /* Default value cannot be changed */
+ if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL)
+ return -EINVAL;
+
+ ret = uvc_ctrl_check_access(chain, ctrls, false);
+ if (ret < 0)
+ return ret;
return uvc_ioctl_s_try_ext_ctrls(handle, ctrls, true);
}
@@ -1130,6 +1159,10 @@ static int uvc_ioctl_try_ext_ctrls(struct file *file,
void *fh,
{
struct uvc_fh *handle = fh;
+ /* Default value cannot be changed */
+ if (ctrls->which == V4L2_CTRL_WHICH_DEF_VAL)
+ return -EINVAL;
+
return uvc_ioctl_s_try_ext_ctrls(handle, ctrls, false);
}
--
2.31.0.rc2.261.g7f71774620-goog
