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

[Dan Scally <dan.scally@ideasonboard.com>]

On 29/11/2022 10:23, Michael Grzeschik wrote:
> On Tue, Nov 29, 2022 at 05:10:24AM +0200, Laurent Pinchart wrote:
>> Hi Michael,
>>
>> (CC'ing Dan)
>>
>> Thank you for the patch.
>>
>> 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.


I'm also not really sure of the benefit; wouldn't this result in 
userspace streaming data that's configured differently to what the host 
is expecting?

>>>  static int
>>>  uvc_send_response(struct uvc_device *uvc, struct uvc_request_data 
>>> *data)
>>>  {
>>> @@ -192,6 +253,21 @@ uvc_send_response(struct uvc_device *uvc, 
>>> struct uvc_request_data *data)
>>>
>>>      memcpy(req->buf, data->data, req->length);
>>>
>>> +    /* validate the ctrl content and fixup */
>>> +    if (!uvc->event_setup_out) {
>>> +        struct uvc_streaming_control *ctrl = req->buf;
>>> +
>>> +        switch (uvc->streaming_request) {
>>> +        case UVC_GET_CUR:
>>> +        case UVC_GET_MIN:
>>> +        case UVC_GET_MAX:
>>> +        case UVC_GET_DEF:
>>> +            uvc_validate_streaming_ctrl(uvc, ctrl);
>>> +        default:
>>> +            break;
>>> +        }
>>> +    }
>>> +


What about read requests for controls that aren't for the streaming 
interface?

>>>      return usb_ep_queue(cdev->gadget->ep0, req, GFP_KERNEL);
>>>  }
>>>
>>
>> -- 
>> Regards,
>>
>> Laurent Pinchart
>>
>