drivers/media/usb/uvc/uvc_ctrl.c | 11 +++++++++-- drivers/media/usb/uvc/uvc_v4l2.c | 9 ++++++--- drivers/media/usb/uvc/uvcvideo.h | 2 +- 3 files changed, 16 insertions(+), 6 deletions(-)
The VIDIOC_G_EXT_CTRLS with which V4L2_CTRL_WHICH_(MIN|MAX)_VAL can only
work for controls that have previously announced support for it.
This patch fixes the following v4l2-compliance error:
info: checking extended control 'User Controls' (0x00980001)
fail: v4l2-test-controls.cpp(980): ret != EINVAL (got 13)
test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL
Fixes: 39d2c891c96e ("media: uvcvideo: support V4L2_CTRL_WHICH_MIN/MAX_VAL")
Cc: stable@vger.kernel.org
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
drivers/media/usb/uvc/uvc_ctrl.c | 11 +++++++++--
drivers/media/usb/uvc/uvc_v4l2.c | 9 ++++++---
drivers/media/usb/uvc/uvcvideo.h | 2 +-
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index 2905505c240c060e5034ea12d33b59d5702f2e1f..2f7d5cdd18e072a47fb5906da0f847dd449911b4 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -1432,7 +1432,7 @@ static bool uvc_ctrl_is_readable(u32 which, struct uvc_control *ctrl,
* auto_exposure=1, exposure_time_absolute=251.
*/
int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
- const struct v4l2_ext_controls *ctrls,
+ const struct v4l2_ext_controls *ctrls, u32 which,
unsigned long ioctl)
{
struct uvc_control_mapping *master_map = NULL;
@@ -1442,14 +1442,21 @@ int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
s32 val;
int ret;
int i;
+ bool is_which_min_max = (ioctl == VIDIOC_G_EXT_CTRLS &&
+ (which == V4L2_CTRL_WHICH_MIN_VAL ||
+ which == V4L2_CTRL_WHICH_MAX_VAL));
if (__uvc_query_v4l2_class(chain, v4l2_id, 0) >= 0)
- return -EACCES;
+ return is_which_min_max ? -EINVAL : -EACCES;
ctrl = uvc_find_control(chain, v4l2_id, &mapping);
if (!ctrl)
return -EINVAL;
+ if ((!(ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX) ||
+ !(ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN)) && is_which_min_max)
+ return -EINVAL;
+
if (ioctl == VIDIOC_G_EXT_CTRLS)
return uvc_ctrl_is_readable(ctrls->which, ctrl, mapping);
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 9e4a251eca88085a1b4e0e854370015855be92ee..d5274dc94da3c60f1f4566b307dd445f30c4f45f 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -765,6 +765,7 @@ static int uvc_ioctl_query_ext_ctrl(struct file *file, void *priv,
static int uvc_ctrl_check_access(struct uvc_video_chain *chain,
struct v4l2_ext_controls *ctrls,
+ u32 which,
unsigned long ioctl)
{
struct v4l2_ext_control *ctrl = ctrls->controls;
@@ -772,7 +773,8 @@ 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, ctrls, ioctl);
+ ret = uvc_ctrl_is_accessible(chain, ctrl->id, ctrls, which,
+ ioctl);
if (ret)
break;
}
@@ -806,7 +808,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *priv,
which = V4L2_CTRL_WHICH_CUR_VAL;
}
- ret = uvc_ctrl_check_access(chain, ctrls, VIDIOC_G_EXT_CTRLS);
+ ret = uvc_ctrl_check_access(chain, ctrls, which, VIDIOC_G_EXT_CTRLS);
if (ret < 0)
return ret;
@@ -840,7 +842,8 @@ static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle,
if (!ctrls->count)
return 0;
- ret = uvc_ctrl_check_access(chain, ctrls, ioctl);
+ ret = uvc_ctrl_check_access(chain, ctrls, V4L2_CTRL_WHICH_CUR_VAL,
+ ioctl);
if (ret < 0)
return ret;
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index ed7bad31f75ca474c1037d666d5310c78dd764df..d583425893a5f716185153a07aae9bfe20182964 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -786,7 +786,7 @@ int uvc_ctrl_get(struct uvc_video_chain *chain, u32 which,
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,
- const struct v4l2_ext_controls *ctrls,
+ const struct v4l2_ext_controls *ctrls, u32 which,
unsigned long ioctl);
int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
---
base-commit: c218ce4f98eccf5a40de64c559c52d61e9cc78ee
change-id: 20251028-uvc-fix-which-c3ba1fb68ed5
Best regards,
--
Ricardo Ribalda <ribalda@chromium.org>Hi Ricardo,
Thank you for the patch.
On Tue, Oct 28, 2025 at 05:55:10PM +0000, Ricardo Ribalda wrote:
> The VIDIOC_G_EXT_CTRLS with which V4L2_CTRL_WHICH_(MIN|MAX)_VAL can only
> work for controls that have previously announced support for it.
>
> This patch fixes the following v4l2-compliance error:
>
> info: checking extended control 'User Controls' (0x00980001)
> fail: v4l2-test-controls.cpp(980): ret != EINVAL (got 13)
> test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL
>
> Fixes: 39d2c891c96e ("media: uvcvideo: support V4L2_CTRL_WHICH_MIN/MAX_VAL")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 11 +++++++++--
> drivers/media/usb/uvc/uvc_v4l2.c | 9 ++++++---
> drivers/media/usb/uvc/uvcvideo.h | 2 +-
> 3 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 2905505c240c060e5034ea12d33b59d5702f2e1f..2f7d5cdd18e072a47fb5906da0f847dd449911b4 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1432,7 +1432,7 @@ static bool uvc_ctrl_is_readable(u32 which, struct uvc_control *ctrl,
> * auto_exposure=1, exposure_time_absolute=251.
> */
> int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
> - const struct v4l2_ext_controls *ctrls,
> + const struct v4l2_ext_controls *ctrls, u32 which,
> unsigned long ioctl)
> {
> struct uvc_control_mapping *master_map = NULL;
> @@ -1442,14 +1442,21 @@ int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
> s32 val;
> int ret;
> int i;
> + bool is_which_min_max = (ioctl == VIDIOC_G_EXT_CTRLS &&
Is the ioctl check needed, given that this function will be called with
which set to V4L2_CTRL_WHICH_CUR_VAL if ioctl is not VIDIOC_G_EXT_CTRLS
?
> + (which == V4L2_CTRL_WHICH_MIN_VAL ||
> + which == V4L2_CTRL_WHICH_MAX_VAL));
Let's move this up to have longer lines first.
>
> if (__uvc_query_v4l2_class(chain, v4l2_id, 0) >= 0)
> - return -EACCES;
> + return is_which_min_max ? -EINVAL : -EACCES;
>
> ctrl = uvc_find_control(chain, v4l2_id, &mapping);
> if (!ctrl)
> return -EINVAL;
>
> + if ((!(ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX) ||
> + !(ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN)) && is_which_min_max)
Please put MIN before MAX.
Do we have to bundle min and max, or could we handle them separately ?
Something like
if ((which == V4L2_CTRL_WHICH_MIN_VAL &&
!(ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN)) ||
(which == V4L2_CTRL_WHICH_M1X_VAL &&
!(ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX)))
return -EINVAL;
> + return -EINVAL;
> +
> if (ioctl == VIDIOC_G_EXT_CTRLS)
> return uvc_ctrl_is_readable(ctrls->which, ctrl, mapping);
>
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index 9e4a251eca88085a1b4e0e854370015855be92ee..d5274dc94da3c60f1f4566b307dd445f30c4f45f 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -765,6 +765,7 @@ static int uvc_ioctl_query_ext_ctrl(struct file *file, void *priv,
>
> static int uvc_ctrl_check_access(struct uvc_video_chain *chain,
> struct v4l2_ext_controls *ctrls,
> + u32 which,
This fits on the previous line.
> unsigned long ioctl)
> {
> struct v4l2_ext_control *ctrl = ctrls->controls;
> @@ -772,7 +773,8 @@ 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, ctrls, ioctl);
> + ret = uvc_ctrl_is_accessible(chain, ctrl->id, ctrls, which,
> + ioctl);
> if (ret)
> break;
> }
> @@ -806,7 +808,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *priv,
> which = V4L2_CTRL_WHICH_CUR_VAL;
> }
>
> - ret = uvc_ctrl_check_access(chain, ctrls, VIDIOC_G_EXT_CTRLS);
> + ret = uvc_ctrl_check_access(chain, ctrls, which, VIDIOC_G_EXT_CTRLS);
> if (ret < 0)
> return ret;
>
> @@ -840,7 +842,8 @@ static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle,
> if (!ctrls->count)
> return 0;
>
> - ret = uvc_ctrl_check_access(chain, ctrls, ioctl);
> + ret = uvc_ctrl_check_access(chain, ctrls, V4L2_CTRL_WHICH_CUR_VAL,
> + ioctl);
> if (ret < 0)
> return ret;
>
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index ed7bad31f75ca474c1037d666d5310c78dd764df..d583425893a5f716185153a07aae9bfe20182964 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -786,7 +786,7 @@ int uvc_ctrl_get(struct uvc_video_chain *chain, u32 which,
> 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,
> - const struct v4l2_ext_controls *ctrls,
> + const struct v4l2_ext_controls *ctrls, u32 which,
> unsigned long ioctl);
>
> int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
>
> ---
> base-commit: c218ce4f98eccf5a40de64c559c52d61e9cc78ee
> change-id: 20251028-uvc-fix-which-c3ba1fb68ed5
--
Regards,
Laurent Pinchart
Hi Laurent
On Thu, 13 Nov 2025 at 22:59, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> Thank you for the patch.
>
> On Tue, Oct 28, 2025 at 05:55:10PM +0000, Ricardo Ribalda wrote:
> > The VIDIOC_G_EXT_CTRLS with which V4L2_CTRL_WHICH_(MIN|MAX)_VAL can only
> > work for controls that have previously announced support for it.
> >
> > This patch fixes the following v4l2-compliance error:
> >
> > info: checking extended control 'User Controls' (0x00980001)
> > fail: v4l2-test-controls.cpp(980): ret != EINVAL (got 13)
> > test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL
> >
> > Fixes: 39d2c891c96e ("media: uvcvideo: support V4L2_CTRL_WHICH_MIN/MAX_VAL")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> > drivers/media/usb/uvc/uvc_ctrl.c | 11 +++++++++--
> > drivers/media/usb/uvc/uvc_v4l2.c | 9 ++++++---
> > drivers/media/usb/uvc/uvcvideo.h | 2 +-
> > 3 files changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index 2905505c240c060e5034ea12d33b59d5702f2e1f..2f7d5cdd18e072a47fb5906da0f847dd449911b4 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -1432,7 +1432,7 @@ static bool uvc_ctrl_is_readable(u32 which, struct uvc_control *ctrl,
> > * auto_exposure=1, exposure_time_absolute=251.
> > */
> > int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
> > - const struct v4l2_ext_controls *ctrls,
> > + const struct v4l2_ext_controls *ctrls, u32 which,
> > unsigned long ioctl)
> > {
> > struct uvc_control_mapping *master_map = NULL;
> > @@ -1442,14 +1442,21 @@ int uvc_ctrl_is_accessible(struct uvc_video_chain *chain, u32 v4l2_id,
> > s32 val;
> > int ret;
> > int i;
> > + bool is_which_min_max = (ioctl == VIDIOC_G_EXT_CTRLS &&
>
> Is the ioctl check needed, given that this function will be called with
> which set to V4L2_CTRL_WHICH_CUR_VAL if ioctl is not VIDIOC_G_EXT_CTRLS
Right now it is not needed. But I wanted to be on the protective side
here. Let me add a comment and remove the ioctl check.
> ?
>
> > + (which == V4L2_CTRL_WHICH_MIN_VAL ||
> > + which == V4L2_CTRL_WHICH_MAX_VAL));
>
> Let's move this up to have longer lines first.
>
> >
> > if (__uvc_query_v4l2_class(chain, v4l2_id, 0) >= 0)
> > - return -EACCES;
> > + return is_which_min_max ? -EINVAL : -EACCES;
> >
> > ctrl = uvc_find_control(chain, v4l2_id, &mapping);
> > if (!ctrl)
> > return -EINVAL;
> >
> > + if ((!(ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX) ||
> > + !(ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN)) && is_which_min_max)
>
> Please put MIN before MAX.
>
> Do we have to bundle min and max, or could we handle them separately ?
> Something like
I believe we have to bundle min and max. Controls that support
VIDIOC_G_EXT_CTRLS with V4L2_CTRL_WHICH_(MIN|MAX)_VAL have this flag:
V4L2_CTRL_FLAG_HAS_WHICH_MIN_MAX
```
Whether a control supports querying the minimum and maximum values
using V4L2_CTRL_WHICH_MIN_VAL and V4L2_CTRL_WHICH_MAX_VAL is indicated
by the V4L2_CTRL_FLAG_HAS_WHICH_MIN_MAX flag.
```
>
> if ((which == V4L2_CTRL_WHICH_MIN_VAL &&
> !(ctrl->info.flags & UVC_CTRL_FLAG_GET_MIN)) ||
> (which == V4L2_CTRL_WHICH_M1X_VAL &&
> !(ctrl->info.flags & UVC_CTRL_FLAG_GET_MAX)))
> return -EINVAL;
>
> > + return -EINVAL;
> > +
> > if (ioctl == VIDIOC_G_EXT_CTRLS)
> > return uvc_ctrl_is_readable(ctrls->which, ctrl, mapping);
> >
> > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> > index 9e4a251eca88085a1b4e0e854370015855be92ee..d5274dc94da3c60f1f4566b307dd445f30c4f45f 100644
> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > @@ -765,6 +765,7 @@ static int uvc_ioctl_query_ext_ctrl(struct file *file, void *priv,
> >
> > static int uvc_ctrl_check_access(struct uvc_video_chain *chain,
> > struct v4l2_ext_controls *ctrls,
> > + u32 which,
>
> This fits on the previous line.
>
> > unsigned long ioctl)
> > {
> > struct v4l2_ext_control *ctrl = ctrls->controls;
> > @@ -772,7 +773,8 @@ 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, ctrls, ioctl);
> > + ret = uvc_ctrl_is_accessible(chain, ctrl->id, ctrls, which,
> > + ioctl);
> > if (ret)
> > break;
> > }
> > @@ -806,7 +808,7 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void *priv,
> > which = V4L2_CTRL_WHICH_CUR_VAL;
> > }
> >
> > - ret = uvc_ctrl_check_access(chain, ctrls, VIDIOC_G_EXT_CTRLS);
> > + ret = uvc_ctrl_check_access(chain, ctrls, which, VIDIOC_G_EXT_CTRLS);
> > if (ret < 0)
> > return ret;
> >
> > @@ -840,7 +842,8 @@ static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle,
> > if (!ctrls->count)
> > return 0;
> >
> > - ret = uvc_ctrl_check_access(chain, ctrls, ioctl);
> > + ret = uvc_ctrl_check_access(chain, ctrls, V4L2_CTRL_WHICH_CUR_VAL,
> > + ioctl);
> > if (ret < 0)
> > return ret;
> >
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index ed7bad31f75ca474c1037d666d5310c78dd764df..d583425893a5f716185153a07aae9bfe20182964 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -786,7 +786,7 @@ int uvc_ctrl_get(struct uvc_video_chain *chain, u32 which,
> > 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,
> > - const struct v4l2_ext_controls *ctrls,
> > + const struct v4l2_ext_controls *ctrls, u32 which,
> > unsigned long ioctl);
> >
> > int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
> >
> > ---
> > base-commit: c218ce4f98eccf5a40de64c559c52d61e9cc78ee
> > change-id: 20251028-uvc-fix-which-c3ba1fb68ed5
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
© 2016 - 2026 Red Hat, Inc.