[PATCH] usb: cdnsp: drop runtime PM reference on gadget init failure

[PATCH] usb: cdnsp: drop runtime PM reference on gadget init failure

[Wentao Liang]

__cdnsp_gadget_init() takes a runtime PM reference with
pm_runtime_get_sync().  On success the reference is dropped by
cdnsp_gadget_exit(), which the role core only calls once the role state
has been set to active.  The failure paths return before that, so the
reference is leaked.

Drop the reference on the common error path.

Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
Cc: stable@vger.kernel.org
Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/usb/cdns3/cdnsp-gadget.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
index e84405352caa..d3df4bc4a17b 100644
--- a/drivers/usb/cdns3/cdnsp-gadget.c
+++ b/drivers/usb/cdns3/cdnsp-gadget.c
@@ -2083,6 +2083,7 @@ static int __cdnsp_gadget_init(struct cdns *cdns)
 free_setup:
 	kfree(pdev->setup_buf);
 free_pdev:
+	pm_runtime_put_autosuspend(cdns->dev);
 	kfree(pdev);
 
 	return ret;
-- 
2.34.1

Re: [PATCH] usb: cdnsp: drop runtime PM reference on gadget init failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] usb: cdnsp: dangling cdns->gadget_dev on __cdnsp_gadget_init() failure
--

commit 70891c067700e0e2a381cc692d3a9164919460f6
Author: Wentao Liang <vulab@iscas.ac.cn>

usb: cdnsp: drop runtime PM reference on gadget init failure

This commit adds pm_runtime_put_autosuspend(cdns->dev) to the free_pdev error
path in __cdnsp_gadget_init() to drop the runtime PM reference acquired via
pm_runtime_get_sync(cdns->dev) when gadget initialization fails.

> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
> index e84405352caab..d3df4bc4a17bc 100644
> --- a/drivers/usb/cdns3/cdnsp-gadget.c
> +++ b/drivers/usb/cdns3/cdnsp-gadget.c
> @@ -2083,6 +2083,7 @@ static int __cdnsp_gadget_init(struct cdns *cdns)
>  free_setup:
>  	kfree(pdev->setup_buf);
>  free_pdev:
> +	pm_runtime_put_autosuspend(cdns->dev);
>  	kfree(pdev);

[Severity: Critical]
When __cdnsp_gadget_init() fails and jumps to free_pdev, does dropping the
runtime PM reference and freeing pdev here expose a use-after-free on
cdns->gadget_dev during subsequent suspend or resume?

Before the fallible initialization steps (allocating pdev->setup_buf,
cdnsp_gen_setup(), cdnsp_gadget_init_endpoints(), usb_add_gadget_udc(), and
devm_request_threaded_irq()), __cdnsp_gadget_init() enables the DRD gadget and
stores pdev in cdns->gadget_dev:

drivers/usb/cdns3/cdnsp-gadget.c:__cdnsp_gadget_init()
	cdns_drd_gadget_on(cdns);

	pdev = kzalloc_obj(*pdev);
	if (!pdev)
		return -ENOMEM;

	pm_runtime_get_sync(cdns->dev);

	cdns->gadget_dev = pdev;

And in drivers/usb/cdns3/core.c, cdns_role_start() sets cdns->role =
USB_ROLE_DEVICE before calling start(), leaving cdns->role set to
USB_ROLE_DEVICE with state CDNS_ROLE_STATE_INACTIVE when start() fails:

drivers/usb/cdns3/core.c:cdns_role_start()
	mutex_lock(&cdns->mutex);
	cdns->role = role;
	mutex_unlock(&cdns->mutex);
	...
	mutex_lock(&cdns->mutex);
	ret = cdns->roles[role]->start(cdns);
	if (!ret)
		cdns->roles[role]->state = CDNS_ROLE_STATE_ACTIVE;
	mutex_unlock(&cdns->mutex);

Because the role state remains inactive, cdnsp_gadget_exit() is not called,
and the free_pdev path neither clears cdns->gadget_dev = NULL nor calls
cdns_drd_gadget_off(cdns).

With pm_runtime_put_autosuspend(cdns->dev) now allowing the device to enter
runtime suspend, a subsequent runtime resume or system suspend/resume will
call cdns_suspend() or cdns_resume(), which invoke
cdns->roles[cdns->role]->suspend() (cdnsp_gadget_suspend()) and
cdns->roles[cdns->role]->resume() (cdnsp_gadget_resume()) without checking if
the role state is active:

drivers/usb/cdns3/cdnsp-gadget.c:cdnsp_gadget_suspend()
	struct cdnsp_device *pdev = cdns->gadget_dev;
	unsigned long flags;

	spin_lock_irqsave(&pdev->lock, flags);
	cdnsp_disconnect_gadget(pdev);
	cdnsp_stop(pdev);

drivers/usb/cdns3/cdnsp-gadget.c:cdnsp_gadget_resume()
	struct cdnsp_device *pdev = cdns->gadget_dev;
	...
	if (!pdev->gadget_driver)
		return 0;

Both functions dereference the freed cdns->gadget_dev pointer. Should the
error path in __cdnsp_gadget_init() also clear cdns->gadget_dev = NULL and
call cdns_drd_gadget_off(cdns) (matching cdnsp_gadget_exit()), or should
cdns_suspend() and cdns_resume() check whether the role is active before
invoking the role callbacks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104021503.1468519-1-vulab@iscas.ac.cn?part=1

Re: [PATCH] usb: cdnsp: drop runtime PM reference on gadget init failure

[Peter Chen]

On 26-11-04 02:15:03, Wentao Liang wrote:
> __cdnsp_gadget_init() takes a runtime PM reference with
> pm_runtime_get_sync().  On success the reference is dropped by
> cdnsp_gadget_exit(), which the role core only calls once the role state
> has been set to active.  The failure paths return before that, so the
> reference is leaked.
> 
> Drop the reference on the common error path.
> 
> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> Cc: stable@vger.kernel.org
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> ---
>  drivers/usb/cdns3/cdnsp-gadget.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
> index e84405352caa..d3df4bc4a17b 100644
> --- a/drivers/usb/cdns3/cdnsp-gadget.c
> +++ b/drivers/usb/cdns3/cdnsp-gadget.c
> @@ -2083,6 +2083,7 @@ static int __cdnsp_gadget_init(struct cdns *cdns)
>  free_setup:
>  	kfree(pdev->setup_buf);
>  free_pdev:
> +	pm_runtime_put_autosuspend(cdns->dev);
>  	kfree(pdev);
>  
>  	return ret;
> -- 
> 2.34.1
> 
> 

-- 

Thanks,
Peter Chen

Re: [PATCH] usb: cdnsp: drop runtime PM reference on gadget init failure

[Krzysztof Kozlowski]

On 04/11/2026 09:03, Peter Chen wrote:
> On 26-11-04 02:15:03, Wentao Liang wrote:
>> __cdnsp_gadget_init() takes a runtime PM reference with
>> pm_runtime_get_sync().  On success the reference is dropped by
>> cdnsp_gadget_exit(), which the role core only calls once the role state
>> has been set to active.  The failure paths return before that, so the
>> reference is leaked.
>>
>> Drop the reference on the common error path.
>>
>> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
>> Cc: stable@vger.kernel.org
>> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
>> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> 
> Acked-by: Peter Chen <peter.chen@kernel.org>

You sure? Sashiko reports introduced UAF. Mind that vulab@iscas.ac.cn is
a bot with spams with tons of patches which looks exactly like previous
hypocrite commit approach - some security vulnerability intentionally
added in between of thousands of valid patches.

Best regards,
Krzysztof

Re: [PATCH] usb: cdnsp: drop runtime PM reference on gadget init failure

[Peter Chen]

On 26-11-04 09:33:29, Krzysztof Kozlowski wrote:
> On 04/11/2026 09:03, Peter Chen wrote:
> > On 26-11-04 02:15:03, Wentao Liang wrote:
> >> __cdnsp_gadget_init() takes a runtime PM reference with
> >> pm_runtime_get_sync().  On success the reference is dropped by
> >> cdnsp_gadget_exit(), which the role core only calls once the role state
> >> has been set to active.  The failure paths return before that, so the
> >> reference is leaked.
> >>
> >> Drop the reference on the common error path.
> >>
> >> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> >> Cc: stable@vger.kernel.org
> >> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> >> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> > 
> > Acked-by: Peter Chen <peter.chen@kernel.org>
> 
> You sure? Sashiko reports introduced UAF. Mind that vulab@iscas.ac.cn is
> a bot with spams with tons of patches which looks exactly like previous
> hypocrite commit approach - some security vulnerability intentionally
> added in between of thousands of valid patches.
> 

Oh, my fault. Thanks for pointing it.

-- 

Thanks,
Peter Chen

Re: [PATCH] usb: cdnsp: drop runtime PM reference on gadget init failure

[Peter Chen]

On 26-11-04 00:03:34, Peter Chen wrote:
> On 26-11-04 02:15:03, Wentao Liang wrote:
> > __cdnsp_gadget_init() takes a runtime PM reference with
> > pm_runtime_get_sync().  On success the reference is dropped by
> > cdnsp_gadget_exit(), which the role core only calls once the role state
> > has been set to active.  The failure paths return before that, so the
> > reference is leaked.
> > 
> > Drop the reference on the common error path.
> > 
> > Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> > Cc: stable@vger.kernel.org
> > Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> > Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> 
> Acked-by: Peter Chen <peter.chen@kernel.org>


Hi Greg,

Please drop my ack, I did not consider the situations bot reported.

Peter

> > ---
> >  drivers/usb/cdns3/cdnsp-gadget.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
> > index e84405352caa..d3df4bc4a17b 100644
> > --- a/drivers/usb/cdns3/cdnsp-gadget.c
> > +++ b/drivers/usb/cdns3/cdnsp-gadget.c
> > @@ -2083,6 +2083,7 @@ static int __cdnsp_gadget_init(struct cdns *cdns)
> >  free_setup:
> >  	kfree(pdev->setup_buf);
> >  free_pdev:
> > +	pm_runtime_put_autosuspend(cdns->dev);
> >  	kfree(pdev);
> >  
> >  	return ret;
> > -- 
> > 2.34.1
> > 
> > 
> 
> -- 
> 
> Thanks,
> Peter Chen

-- 

Thanks,
Peter Chen