Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <d08b080c-54cc-4fdf-929a-df2b6ad41844@google.com>
Date:   Wed, 1 Nov 2023 15:13:58 -0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v10 2/4] usb: gadget: uvc: Allocate uvc_requests one at a
 time
Content-Language: en-US
To:     Dan Scally <dan.scally@ideasonboard.com>
Cc:     etalvala@google.com, gregkh@linuxfoundation.org,
        jchowdhary@google.com, laurent.pinchart@ideasonboard.com,
        linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
        m.grzeschik@pengutronix.de
References: <73309396-3856-43a2-9a6f-81a40ed594db@google.com>
 <20231030202231.3263253-1-arakesh@google.com>
 <20231030202231.3263253-2-arakesh@google.com>
 <7d4c762a-9fe1-41ba-b394-cefa8fa70786@ideasonboard.com>
From:   Avichal Rakesh <arakesh@google.com>
In-Reply-To: <7d4c762a-9fe1-41ba-b394-cefa8fa70786@ideasonboard.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 11/1/23 04:06, Dan Scally wrote:
> Morning Avichal
> 
> On 30/10/2023 20:22, 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 patch is 1 of 2 patches addressing the use-after-free issue.
>> Instead of bulk allocating all uvc_requests as an array, this patch
>> allocates uvc_requests one at a time, which should allows for similar
>> granularity when deallocating the uvc_requests. This patch has no
>> functional changes other than allocating each uvc_request separately,
>> and similarly freeing each of them separately.
>>
>> Link: https://lore.kernel.org/7cd81649-2795-45b6-8c10-b7df1055020d@google.com
>> Suggested-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> Reviewed-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> Tested-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> Signed-off-by: Avichal Rakesh <arakesh@google.com>
> 
> 
> Thanks for the update; this seems ok now:
> 
> 
> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

Awesome, thank you! I'll add the Reviewed-by in the next patchset
(assuming you have more review comments on patch 4/4 v10). 

Regards,
Avi.

> 
>> ---
>> v1 -> v2 : Rebased to ToT
>> v2 -> v3 : Fix email threading goof-up
>> v3 -> v4 : Address review comments & re-rebase to ToT
>> v4 -> v5 : Address more review comments. Add Reviewed-by & Tested-by.
>> v5 -> v6 : No change
>> v6 -> v7 : No change
>> v7 -> v8 : No change. Getting back in review queue
>> v8 -> v9 : Address review comments.
>> v9 -> v10: Address review comments; remove BUG_ON(&video->reqs);
>>             Rebase to ToT (usb-next)
>>
>>   <snip>