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

[Avichal Rakesh]

On 11/8/23 06:15, Dan Scally wrote:
> Hi Avichal
> 
> On 02/11/2023 20:19, 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 accommodate 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
>> Reviewed-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
>> Suggested-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
>> Tested-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
>> Signed-off-by: Avichal Rakesh <arakesh@xxxxxxxxxx>
>> ---
> 
> 
> Thanks for the update. Let's leave the locking as it is; I think albeit not strictly necessary on that occasion it certainly is necessary to take the lock to protect the flags elsewhere, and probably better to be consistent with it.
> 
> 
> Reviewed-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>

Thank you for reviewing, Dan!

Greg, I just sent out v12 with the Reviewed-by tag:
https://lore.kernel.org/all/20231109004104.3467968-1-arakesh@xxxxxxxxxx/ 
They should be ready to submit now. Thank you!

Regards,
Avi.

> 
>> <snip>