linux-kernel - Re: [PATCH v15 14/19] media: uvcvideo: Use the camera to clamp compound controls

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1cac4857-c261-461f-9749-8c8c68a0db07@redhat.com>
Date: Mon, 9 Dec 2024 15:05:41 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Ricardo Ribalda <ribalda@...omium.org>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Mauro Carvalho Chehab <mchehab@...nel.org>,
 Ricardo Ribalda <ribalda@...nel.org>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>,
 Hans Verkuil <hverkuil@...all.nl>
Cc: Yunke Cao <yunkec@...omium.org>, linux-media@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v15 14/19] media: uvcvideo: Use the camera to clamp
 compound controls

Hi,

On 14-Nov-24 8:10 PM, Ricardo Ribalda wrote:
> Compound controls cannot e reliable clamped. There is plenty of space
> for interpretation for the device manufacturer.
> 
> When we write a compound control, let the camera do the clamping and
> return back to the user the value used by the device.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@...omium.org>
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 0dae5e8c3ca0..72ed7dc9cfc1 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2339,6 +2339,18 @@ int uvc_ctrl_set(struct uvc_fh *handle, struct v4l2_ext_control *xctrl)
>  
>  	ctrl->dirty = 1;
>  	ctrl->modified = 1;
> +
> +	/*
> +	 * Compound controls cannot reliable clamp the value when they are
> +	 * written to the device. Let the device do the clamping and read back
> +	 * the value that the device is using. We do not need to return an
> +	 * error if this fails.
> +	 */
> +	if (uvc_ctrl_mapping_is_compound(mapping) &&
> +	    uvc_ctrl_is_readable(V4L2_CTRL_WHICH_CUR_VAL, ctrl, mapping))
> +		uvc_mapping_get_xctrl_compound(chain, ctrl, mapping,
> +					       V4L2_CTRL_WHICH_CUR_VAL, xctrl);
> +

I do not believe that this actually works / does what you want it to do.

At this point we have only updated in memory structures for the control
and not send anything to camera.

Querying the control to return the actual achieved values to userspace
only makes sense after uvc_ctrl_commit() has succeeded, unless I am
missing something ?

Regards,

Hans