Re: [PATCH v7] usb: gadget: uvc: add validate and fix function for uvc response

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 29, 2022 at 04:22:59PM +0100, Michael Grzeschik wrote:
On Tue, Nov 29, 2022 at 02:02:02PM +0200, Laurent Pinchart wrote:
On Tue, Nov 29, 2022 at 11:23:08AM +0100, Michael Grzeschik wrote:
On Tue, Nov 29, 2022 at 05:10:24AM +0200, Laurent Pinchart wrote:
On Mon, Nov 28, 2022 at 11:31:25AM +0100, Michael Grzeschik wrote:
When the userspace gets the setup requests for UVC_GET_CUR UVC_GET_MIN,
UVC_GET_MAX, UVC_GET_DEF it will fill out the ctrl response. This data
needs to be validated. Since the kernel also knows the limits for valid
cases, it can fixup the values in case the userspace is setting invalid
data.

Why is this a good idea ?

Why is it not? We don't want the userspace to communicate other things
to the host than what is configured in the configfs. If you only object
the explanation, then I will improve the commit message and send an
fixed v8. If you have more objections please share your doubts, thanks.

What bothers me is that this patch silently clamps invalid value, trying
to hide the gadget userspace error from the host. It may allow the host
to proceed one step further, but if the gadget userspace got it wrong in
the first place, there's a very high chance it won't do the right thing
in the next step anyway. This will make debugging more complicated,
while at the same time not bringing much value.

I discussed this and we came up with a better approach. When the
userspace will send UVCIOC_SEND_RESPONSE we can return with a negativ
return value. Like EAGAIN if the validation was seeeing some trouble
with the userspaces uvc_streaming_control feedback to the host.

The validation code will then still fixup the data, but instead of
transfering this manipulated answer to the host, it will return the
changes to the application with EAGAIN. So now the userspace can
react to it and it should even point out misconfigurations between
kernel and userspace and so will simplify the debugging.

How about that?

While implementing this I came across the problem that the
UVCIOC_SEND_RESPONSE is handled in the vidioc_default handler.
But for this handler we can not set flag INFO_FL_ALWAYS_COPY like
for common v4l2_ioctls. :(

I think this is still worth a path to go, but I am currently out
of ideas how to achieve it. Help for this is much appreciated.

Thanks,
Michael


--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux