Re: [PATCH] usb: cdns3: fix use-after-free in cdns3_gadget_exit()

[Peter Chen]

On 26-09-09 09:56:55, Fan Wu wrote:
> cdns3_gadget_start() arms two works on system_freezable_wq:
> pending_status_wq for the deferred ep0 status stage and aligned_buf_wq
> for realigned request buffers. Both handlers use the cdns3_device the
> works are embedded in, and cdns3_pending_setup_status_handler() also
> calls the ep0 request completion.
> 
> cdns3_gadget_exit() does not wait for these works. It frees all
> endpoints and aligned buffers and drops the last reference to the
> gadget device, which frees priv_dev, so a work queued before the exit
> can run after the free.
> 
> Fix this by waiting for both works after the gadget driver is unbound
> and the IRQ is freed, when no new work can be queued, and before the
> endpoints and buffers are released.
> 
> This issue was found by an in-house static analysis tool.
> 
> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Sicong Huang <congei42@xxxxxxx>
> Closes: https://lore.kernel.org/linux-usb/7f5719b.8700.18f67b324d3.Coremail.congei42@xxxxxxx/
> Suggested-by: Sicong Huang <congei42@xxxxxxx>
> Assisted-by: Codex:gpt-5.6
> Co-developed-by: Song Li <songl@xxxxxxxxxx>
> Signed-off-by: Song Li <songl@xxxxxxxxxx>
> Signed-off-by: Fan Wu <fanwu01@xxxxxxxxxx>
> ---
>  drivers/usb/cdns3/cdns3-gadget.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
> index 42311c1bf..0d272e9c5 100644
> --- a/drivers/usb/cdns3/cdns3-gadget.c
> +++ b/drivers/usb/cdns3/cdns3-gadget.c
> @@ -3269,6 +3269,10 @@ static void cdns3_gadget_exit(struct cdns *cdns)
>  	usb_del_gadget(&priv_dev->gadget);
>  	devm_free_irq(cdns->dev, cdns->dev_irq, priv_dev);
>  
> +	/* The works can still be queued until the IRQ is freed. */
> +	cancel_work_sync(&priv_dev->pending_status_wq);
> +	cancel_work_sync(&priv_dev->aligned_buf_wq);
> +

We may cancel work before going to de-init. How about move them before

	pm_runtime_put_autosuspend(cdns->dev);

-- 

Thanks,
Peter Chen