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
