| Message ID | 20230911140530.2995138-3-m.grzeschik@pengutronix.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 52a39f2cf62bb5430ad1f54cd522dbfdab1d71ba |
| Headers | show |
| Series | usb: gadget: uvc: restart fixes | expand |
Hi Michael, Thank you for the patch. On Mon, Sep 11, 2023 at 04:05:29PM +0200, 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. You're swapping one race condition for a different one. With this patch, request can now be double-freed. I'll reply to the long discussion on v1 to try and find a proper solution. As for 1/3, I think this got merged too soon :-( > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > --- > v1 == v2 > > 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 4b68a3a9815d73..c48c904f500fff 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); > + ureq->req = NULL; > + return; > + } > + > switch (req->status) { > case 0: > break;
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index 4b68a3a9815d73..c48c904f500fff 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); + ureq->req = NULL; + return; + } + switch (req->status) { case 0: break;
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@pengutronix.de> --- v1 == v2 drivers/usb/gadget/function/uvc_video.c | 6 ++++++ 1 file changed, 6 insertions(+)