Re: [PATCH v4 3/3] usb: gadget: uvc: Fix use-after-free for inflight usb_requests

[Avichal Rakesh]

On 10/11/23 17:24, Avichal Rakesh wrote:
> Currently, the uvc gadget driver allocates all uvc_requests as one array
> and deallocates them all when the video stream stops. This includes
> de-allocating all the usb_requests associated with those uvc_requests.
> This can lead to use-after-free issues if any of those de-allocated
> usb_requests were still owned by the usb controller.
> 
> This is patch 2 of 2 in fixing the use-after-free issue. It adds a new
> flag to uvc_video to track when frames and requests should be flowing.
> When disabling the video stream, the flag is tripped and, instead
> of de-allocating all uvc_requests and usb_requests, the gadget
> driver only de-allocates those usb_requests that are currently
> owned by it (as present in req_free). Other usb_requests are left
> untouched until their completion handler is called which takes care
> of freeing the usb_request and its corresponding uvc_request.
> 
> Now that uvc_video does not depends on uvc->state, this patch removes
> unnecessary upates to uvc->state that were made to accomodate uvc_video
> logic. This should ensure that uvc gadget driver never accidentally
> de-allocates a usb_request that it doesn't own.
> 
> Link: https://lore.kernel.org/7cd81649-2795-45b6-8c10-b7df1055020d@xxxxxxxxxx
> Suggested-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> Signed-off-by: Avichal Rakesh <arakesh@xxxxxxxxxx>
> ---
> v1 -> v2: Rebased to ToT, and fixed deadlock reported in
>           https://lore.kernel.org/all/ZRv2UnKztgyqk2pt@xxxxxxxxxxxxxx/
> v2 -> v3: Fix email threading goof-up
> v3 -> v4: re-rebase to ToT & moved to a uvc_video level lock
>           as discussed in
>           https://lore.kernel.org/b14b296f-2e08-4edf-aeea-1c5b621e2d0c@xxxxxxxxxx/
> 
>  drivers/usb/gadget/function/uvc.h       |   1 +
>  drivers/usb/gadget/function/uvc_v4l2.c  |  12 +-
>  drivers/usb/gadget/function/uvc_video.c | 156 +++++++++++++++++++-----
>  3 files changed, 128 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h

Apologies, I realized I forgot to run checkpatch on this patch. Will fix 
the lint issues in the next version. This patch is functionally okay, but 
has 2 minor formatting issues. Feel free to review the patch, and I will
fix the formatting as I am addressing the comments.

- Avi.