Hey Michael,
On 9/10/23 17:24, Michael Grzeschik wrote:
> The uvc_video_enable function of the uvc-gadget driver is dequeing and
> immediately deallocs all requests on its disable codepath. This is not
> save since the dequeue function is async and does not ensure that the
> requests are left unlinked in the controller driver.
>
> By adding the ep_free_request into the completion path of the requests
> we ensure that the request will be properly deallocated.
>
> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> ---
> drivers/usb/gadget/function/uvc_video.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
> index 4b6e854e30c58c..52e3666b51f743 100644
> --- a/drivers/usb/gadget/function/uvc_video.c
> +++ b/drivers/usb/gadget/function/uvc_video.c
> @@ -256,6 +256,12 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
> struct uvc_device *uvc = video->uvc;
> unsigned long flags;
>
> + if (uvc->state == UVC_STATE_CONNECTED) {
> + usb_ep_free_request(video->ep, ureq->req);
nit: You can probably just call usb_ep_free_request with req instead of ureq->req.
> + ureq->req = NULL;
> + return;
> + }
> +
> switch (req->status) {
> case 0:
> break;
Perhaps I am missing something here, but I am not sure how this alone
fixes the use-after-free issue. uvcg_video_enable still deallocates
_all_ usb_requests right after calling usb_ep_dequeue, so it is still
possible that an unreturned request is deallocated, and now it is
possible that the complete callback accesses a deallocated ureq :(
Regards,
Avi
