[PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition

[PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition

[Pei Xiao]

In rcar_gen3_phy_usb2_probe, &channel->work is bound with
rcar_gen3_phy_usb2_work. rcar_gen3_phy_usb2_irq can schedule this work
on system_wq via rcar_gen3_device_recognition(), and the role sysfs
store can also schedule it via rcar_gen3_init_for_host() /
rcar_gen3_init_for_peri().

If we remove the device, rcar_gen3_phy_usb2_remove makes cleanup and
the memory allocated for channel with devm_kzalloc() is released by
the devm cleanup after the remove callback returns, while the work
mentioned above may still be pending or running. The sequence of
operations that may lead to a UAF bug is as follows:

CPU0                                      CPU1

                                          | rcar_gen3_phy_usb2_irq
                                          | rcar_gen3_device_recognition
                                          | rcar_gen3_init_for_host
                                          | schedule_work(&ch->work)
rcar_gen3_phy_usb2_remove                 |
device_remove_file(&pdev->dev,            |
                   &dev_attr_role)        |
// remove returns                         |
// devm cleanup: free_irq,                |
// kfree(channel)                         |
                                          | rcar_gen3_phy_usb2_work
                                          | // use ch (use-after-free)

Fix it by disabling the OTG interrupts, so the IRQ handler cannot
schedule new work, and canceling the work before the remaining cleanup
in rcar_gen3_phy_usb2_remove and the devm release of channel.

Fixes: c14f8a4032ef ("phy: rcar-gen3-usb2: fix mutex_lock calling in interrupt")
Assisted-by: Codex:deepseek-v4-flash
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/phy/renesas/phy-rcar-gen3-usb2.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
index 9a45d840efeb..fa0e680a4b91 100644
--- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
+++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
@@ -1067,8 +1067,16 @@ static void rcar_gen3_phy_usb2_remove(struct platform_device *pdev)
 {
 	struct rcar_gen3_chan *channel = platform_get_drvdata(pdev);
 
-	if (channel->is_otg_channel)
+	if (channel->is_otg_channel) {
+		/* Disable OTG interrupts so the IRQ handler cannot
+		 * schedule new work.
+		 */
+		rcar_gen3_control_otg_irq(channel, 0);
+
 		device_remove_file(&pdev->dev, &dev_attr_role);
+
+		cancel_work_sync(&channel->work);
+	}
 }
 
 static int rcar_gen3_phy_usb2_suspend(struct device *dev)
-- 
2.25.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Critical] Accessing hardware registers in `remove()` while the device may be runtime suspended causes a synchronous external abort (kernel panic).
- [High] Unlocked hardware register access in `remove()` races with the `role_store()` sysfs operation, allowing interrupts to be re-enabled and completely bypassing the fix.

Pre-existing issues:
- [High] The patch fails to resolve the use-after-free race condition because it does not synchronize with an already-executing IRQ handler before calling `cancel_work_sync()`.
- [High] Missing initialization of `work_struct` when an IRQ is absent causes a kernel panic on sysfs writes.
- [High] Missing `cancel_work_sync()` on probe error paths leads to use-after-free if probe fails.
--

--- Patch [1]: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition ---
commit c1e0683754e769afd34c8d1183cbf96e6d4a4b81
Author: Pei Xiao <xiaopei01@kylinos.cn>

phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition

This commit attempts to fix a use-after-free in the driver remove path by
disabling OTG interrupts and canceling the work queue before the devm cleanup
happens.

> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> index 9a45d840efeb2..fa0e680a4b915 100644
> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> @@ -1067,8 +1067,16 @@ static void rcar_gen3_phy_usb2_remove(struct platform_device *pdev)
>  {
>  	struct rcar_gen3_chan *channel = platform_get_drvdata(pdev);
>  
> -	if (channel->is_otg_channel)
> +	if (channel->is_otg_channel) {
> +		/* Disable OTG interrupts so the IRQ handler cannot
> +		 * schedule new work.
> +		 */
> +		rcar_gen3_control_otg_irq(channel, 0);

[Severity: Critical]
Will this trigger a synchronous external abort?

If there are no active PHY consumers when rcar_gen3_phy_usb2_remove() is
called, the device will be runtime suspended and its clocks gated.

Calling rcar_gen3_control_otg_irq() here reads and writes to USB2_OBINTEN
without first waking the device via pm_runtime_resume_and_get().

[Severity: High]
Does this unlocked register access race with the sysfs role_store() function?

If CPU1 concurrently writes to the sysfs role attribute, role_store() takes
channel->lock and eventually executes rcar_gen3_control_otg_irq(channel, 1).

Because the interrupt is disabled here without channel->lock, CPU1's write
can re-enable the interrupt right before device_remove_file() is called.

This would leave the interrupt fully enabled, bypassing the intended fix.

> +
>  		device_remove_file(&pdev->dev, &dev_attr_role);
> +
> +		cancel_work_sync(&channel->work);

[Severity: High]
This is a pre-existing issue, but does this actually prevent the
use-after-free?

Since the IRQ handler is shared and managed by devres, it isn't freed until
after rcar_gen3_phy_usb2_remove() returns.

If CPU1 is already executing rcar_gen3_phy_usb2_irq(), it might have already
read the status register. CPU0 can disable the IRQ and call cancel_work_sync(),
but CPU1 can subsequently call schedule_work().

When the remove function returns and devres frees the channel, the queued work
will execute and cause a use-after-free. To fix this, the IRQ must be
explicitly freed or synchronized before cancel_work_sync().

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for sysfs writes
to trigger a NULL pointer dereference on the work queue?

In rcar_gen3_phy_usb2_probe():

    if (irq < 0 && irq != -ENXIO) {
            return irq;
    } else if (irq > 0) {
            INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
    }

If irq <= 0, INIT_WORK is skipped. However, if dr_mode is configured,
the role sysfs attribute is still created. Writing to it will call
schedule_work(&channel->work) on an uninitialized work structure.

[Severity: High]
This is a pre-existing issue, but does the probe error path leak pending work?

If devm_request_irq() succeeds in rcar_gen3_phy_usb2_probe(), the shared IRQ
becomes live.

If a subsequent probe step fails (like devm_of_phy_provider_register), the
probe function returns an error and devres frees the IRQ and channel memory.

If the IRQ fired and scheduled channel->work, there is no explicit
cancel_work_sync() on the error path to stop it before the memory is freed,
potentially leading to a use-after-free.

> +	}
>  }
>  
>  static int rcar_gen3_phy_usb2_suspend(struct device *dev)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/d265021fb7fe432e32eedbe9e075fb041c15cbe6.1785830417.git.xiaopei01@kylinos.cn?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

RE: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition

[Biju Das]

Hi Pei Xiao,

Thanks for the patch.

> -----Original Message-----
> From: Pei Xiao <xiaopei01@kylinos.cn>
> Sent: 04 August 2026 09:02
> Subject: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to
> race condition
> 
> In rcar_gen3_phy_usb2_probe, &channel->work is bound with rcar_gen3_phy_usb2_work. rcar_gen3_phy_usb2_irq
> can schedule this work on system_wq via rcar_gen3_device_recognition(), and the role sysfs store can also
> schedule it via rcar_gen3_init_for_host() / rcar_gen3_init_for_peri().
> 
> If we remove the device, rcar_gen3_phy_usb2_remove makes cleanup and the memory allocated for channel
> with devm_kzalloc() is released by the devm cleanup after the remove callback returns, while the work
> mentioned above may still be pending or running. The sequence of operations that may lead to a UAF bug is
> as follows:
> 
> CPU0                                      CPU1
> 
>                                           | rcar_gen3_phy_usb2_irq
>                                           | rcar_gen3_device_recognition
>                                           | rcar_gen3_init_for_host
>                                           | schedule_work(&ch->work)
> rcar_gen3_phy_usb2_remove                 |
> device_remove_file(&pdev->dev,            |
>                    &dev_attr_role)        |
> // remove returns                         |
> // devm cleanup: free_irq,                |
> // kfree(channel)                         |
>                                           | rcar_gen3_phy_usb2_work
>                                           | // use ch (use-after-free)
> 
> Fix it by disabling the OTG interrupts, so the IRQ handler cannot schedule new work, and canceling the
> work before the remaining cleanup in rcar_gen3_phy_usb2_remove and the devm release of channel.
> 
> Fixes: c14f8a4032ef ("phy: rcar-gen3-usb2: fix mutex_lock calling in interrupt")
> Assisted-by: Codex:deepseek-v4-flash
> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
> ---
>  drivers/phy/renesas/phy-rcar-gen3-usb2.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> index 9a45d840efeb..fa0e680a4b91 100644
> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> @@ -1067,8 +1067,16 @@ static void rcar_gen3_phy_usb2_remove(struct platform_device *pdev)  {
>  	struct rcar_gen3_chan *channel = platform_get_drvdata(pdev);
> 
> -	if (channel->is_otg_channel)
> +	if (channel->is_otg_channel) {
> +		/* Disable OTG interrupts so the IRQ handler cannot
> +		 * schedule new work.
> +		 */
> +		rcar_gen3_control_otg_irq(channel, 0);
> +
>  		device_remove_file(&pdev->dev, &dev_attr_role);
> +
> +		cancel_work_sync(&channel->work);

What about pending wq that is still about execute after
"device_remove_file(&pdev->dev, &dev_attr_role);" ?

Cheers,
Biju

> +	}
>  }
> 
>  static int rcar_gen3_phy_usb2_suspend(struct device *dev)
> --
> 2.25.1
> 


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition

[Pei Xiao]

在 2026/8/4 16:34, Biju Das 写道:
> Hi Pei Xiao,
> 
> Thanks for the patch.
> 
>> -----Original Message-----
>> From: Pei Xiao <xiaopei01@kylinos.cn>
>> Sent: 04 August 2026 09:02
>> Subject: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to
>> race condition
>>
>> In rcar_gen3_phy_usb2_probe, &channel->work is bound with rcar_gen3_phy_usb2_work. rcar_gen3_phy_usb2_irq
>> can schedule this work on system_wq via rcar_gen3_device_recognition(), and the role sysfs store can also
>> schedule it via rcar_gen3_init_for_host() / rcar_gen3_init_for_peri().
>>
>> If we remove the device, rcar_gen3_phy_usb2_remove makes cleanup and the memory allocated for channel
>> with devm_kzalloc() is released by the devm cleanup after the remove callback returns, while the work
>> mentioned above may still be pending or running. The sequence of operations that may lead to a UAF bug is
>> as follows:
>>
>> CPU0                                      CPU1
>>
>>                                           | rcar_gen3_phy_usb2_irq
>>                                           | rcar_gen3_device_recognition
>>                                           | rcar_gen3_init_for_host
>>                                           | schedule_work(&ch->work)
>> rcar_gen3_phy_usb2_remove                 |
>> device_remove_file(&pdev->dev,            |
>>                    &dev_attr_role)        |
>> // remove returns                         |
>> // devm cleanup: free_irq,                |
>> // kfree(channel)                         |
>>                                           | rcar_gen3_phy_usb2_work
>>                                           | // use ch (use-after-free)
>>
>> Fix it by disabling the OTG interrupts, so the IRQ handler cannot schedule new work, and canceling the
>> work before the remaining cleanup in rcar_gen3_phy_usb2_remove and the devm release of channel.
>>
>> Fixes: c14f8a4032ef ("phy: rcar-gen3-usb2: fix mutex_lock calling in interrupt")
>> Assisted-by: Codex:deepseek-v4-flash
>> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
>> ---
>>  drivers/phy/renesas/phy-rcar-gen3-usb2.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>> index 9a45d840efeb..fa0e680a4b91 100644
>> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>> @@ -1067,8 +1067,16 @@ static void rcar_gen3_phy_usb2_remove(struct platform_device *pdev)  {
>>  	struct rcar_gen3_chan *channel = platform_get_drvdata(pdev);
>>
>> -	if (channel->is_otg_channel)
>> +	if (channel->is_otg_channel) {
>> +		/* Disable OTG interrupts so the IRQ handler cannot
>> +		 * schedule new work.
>> +		 */
>> +		rcar_gen3_control_otg_irq(channel, 0);
>> +
>>  		device_remove_file(&pdev->dev, &dev_attr_role);
>> +
>> +		cancel_work_sync(&channel->work);
> 
> What about pending wq that is still about execute after
> "device_remove_file(&pdev->dev, &dev_attr_role);" ?
Hi Biju,
  I don't understand what you mean. cancel_work_sync is exactly what
catches this kind of pending work — it either cancels the work that
hasn't run yet, or waits for the one that is currently running to
finish, and it is placed right.
 Could you explain it in more detail?

Thanks！
Pei.

  > after device_remove_file.

> Cheers,
> Biju
> 
>> +	}
>>  }
>>
>>  static int rcar_gen3_phy_usb2_suspend(struct device *dev)
>> --
>> 2.25.1
>>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

RE: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition

[Biju Das]

> -----Original Message-----
> From: Pei Xiao <xiaopei01@kylinos.cn>
> Sent: 04 August 2026 10:05
> To: Biju Das <biju.das.jz@bp.renesas.com>; Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>;
> vkoul@kernel.org; neil.armstrong@linaro.org; geert+renesas@glider.be; magnus.damm
> <magnus.damm@gmail.com>; linux-renesas-soc@vger.kernel.org; linux-phy@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to
> race condition
> 
> 
> 
> 在 2026/8/4 16:34, Biju Das 写道:
> > Hi Pei Xiao,
> >
> > Thanks for the patch.
> >
> >> -----Original Message-----
> >> From: Pei Xiao <xiaopei01@kylinos.cn>
> >> Sent: 04 August 2026 09:02
> >> Subject: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in
> >> rcar_gen3_phy_usb2_remove due to race condition
> >>
> >> In rcar_gen3_phy_usb2_probe, &channel->work is bound with
> >> rcar_gen3_phy_usb2_work. rcar_gen3_phy_usb2_irq can schedule this
> >> work on system_wq via rcar_gen3_device_recognition(), and the role sysfs store can also schedule it
> via rcar_gen3_init_for_host() / rcar_gen3_init_for_peri().
> >>
> >> If we remove the device, rcar_gen3_phy_usb2_remove makes cleanup and
> >> the memory allocated for channel with devm_kzalloc() is released by
> >> the devm cleanup after the remove callback returns, while the work
> >> mentioned above may still be pending or running. The sequence of operations that may lead to a UAF bug
> is as follows:
> >>
> >> CPU0                                      CPU1
> >>
> >>                                           | rcar_gen3_phy_usb2_irq
> >>                                           | rcar_gen3_device_recognition
> >>                                           | rcar_gen3_init_for_host
> >>                                           | schedule_work(&ch->work)
> >> rcar_gen3_phy_usb2_remove                 |
> >> device_remove_file(&pdev->dev,            |
> >>                    &dev_attr_role)        |
> >> // remove returns                         |
> >> // devm cleanup: free_irq,                |
> >> // kfree(channel)                         |
> >>                                           | rcar_gen3_phy_usb2_work
> >>                                           | // use ch
> >> (use-after-free)
> >>
> >> Fix it by disabling the OTG interrupts, so the IRQ handler cannot
> >> schedule new work, and canceling the work before the remaining cleanup in rcar_gen3_phy_usb2_remove
> and the devm release of channel.
> >>
> >> Fixes: c14f8a4032ef ("phy: rcar-gen3-usb2: fix mutex_lock calling in
> >> interrupt")
> >> Assisted-by: Codex:deepseek-v4-flash
> >> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
> >> ---
> >>  drivers/phy/renesas/phy-rcar-gen3-usb2.c | 10 +++++++++-
> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> >> b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> >> index 9a45d840efeb..fa0e680a4b91 100644
> >> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> >> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> >> @@ -1067,8 +1067,16 @@ static void rcar_gen3_phy_usb2_remove(struct platform_device *pdev)  {
> >>  	struct rcar_gen3_chan *channel = platform_get_drvdata(pdev);
> >>
> >> -	if (channel->is_otg_channel)
> >> +	if (channel->is_otg_channel) {
> >> +		/* Disable OTG interrupts so the IRQ handler cannot
> >> +		 * schedule new work.
> >> +		 */
> >> +		rcar_gen3_control_otg_irq(channel, 0);
> >> +
> >>  		device_remove_file(&pdev->dev, &dev_attr_role);
> >> +
> >> +		cancel_work_sync(&channel->work);
> >
> > What about pending wq that is still about execute after
> > "device_remove_file(&pdev->dev, &dev_attr_role);" ?
> Hi Biju,
>   I don't understand what you mean. cancel_work_sync is exactly what catches this kind of pending work —
> it either cancels the work that hasn't run yet, or waits for the one that is currently running to finish,
> and it is placed right.
>  Could you explain it in more detail?

Assume you removed the file, and before cancel_work_sync(), the WQ get scheduled
will it result in UAF bug mentioned in the commit message.

Maybe??

rcar_gen3_control_otg_irq(channel, 0);
cancel_work_sync(&channel->work);
device_remove_file(&pdev->dev, &dev_attr_role);

Cheers,
Biju




> 
> Thanks！
> Pei.
> 
>   > after device_remove_file.
> 
> > Cheers,
> > Biju
> >
> >> +	}
> >>  }
> >>
> >>  static int rcar_gen3_phy_usb2_suspend(struct device *dev)
> >> --
> >> 2.25.1
> >>

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition

[Pei Xiao]

在 2026/8/4 17:08, Biju Das 写道:
> 
> 
>> -----Original Message-----
>> From: Pei Xiao <xiaopei01@kylinos.cn>
>> Sent: 04 August 2026 10:05
>> To: Biju Das <biju.das.jz@bp.renesas.com>; Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>;
>> vkoul@kernel.org; neil.armstrong@linaro.org; geert+renesas@glider.be; magnus.damm
>> <magnus.damm@gmail.com>; linux-renesas-soc@vger.kernel.org; linux-phy@lists.infradead.org; linux-
>> kernel@vger.kernel.org
>> Subject: Re: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to
>> race condition
>>
>>
>>
>> 在 2026/8/4 16:34, Biju Das 写道:
>>> Hi Pei Xiao,
>>>
>>> Thanks for the patch.
>>>
>>>> -----Original Message-----
>>>> From: Pei Xiao <xiaopei01@kylinos.cn>
>>>> Sent: 04 August 2026 09:02
>>>> Subject: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in
>>>> rcar_gen3_phy_usb2_remove due to race condition
>>>>
>>>> In rcar_gen3_phy_usb2_probe, &channel->work is bound with
>>>> rcar_gen3_phy_usb2_work. rcar_gen3_phy_usb2_irq can schedule this
>>>> work on system_wq via rcar_gen3_device_recognition(), and the role sysfs store can also schedule it
>> via rcar_gen3_init_for_host() / rcar_gen3_init_for_peri().
>>>>
>>>> If we remove the device, rcar_gen3_phy_usb2_remove makes cleanup and
>>>> the memory allocated for channel with devm_kzalloc() is released by
>>>> the devm cleanup after the remove callback returns, while the work
>>>> mentioned above may still be pending or running. The sequence of operations that may lead to a UAF bug
>> is as follows:
>>>>
>>>> CPU0                                      CPU1
>>>>
>>>>                                           | rcar_gen3_phy_usb2_irq
>>>>                                           | rcar_gen3_device_recognition
>>>>                                           | rcar_gen3_init_for_host
>>>>                                           | schedule_work(&ch->work)
>>>> rcar_gen3_phy_usb2_remove                 |
>>>> device_remove_file(&pdev->dev,            |
>>>>                    &dev_attr_role)        |
>>>> // remove returns                         |
>>>> // devm cleanup: free_irq,                |
>>>> // kfree(channel)                         |
>>>>                                           | rcar_gen3_phy_usb2_work
>>>>                                           | // use ch
>>>> (use-after-free)
>>>>
>>>> Fix it by disabling the OTG interrupts, so the IRQ handler cannot
>>>> schedule new work, and canceling the work before the remaining cleanup in rcar_gen3_phy_usb2_remove
>> and the devm release of channel.
>>>>
>>>> Fixes: c14f8a4032ef ("phy: rcar-gen3-usb2: fix mutex_lock calling in
>>>> interrupt")
>>>> Assisted-by: Codex:deepseek-v4-flash
>>>> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
>>>> ---
>>>>  drivers/phy/renesas/phy-rcar-gen3-usb2.c | 10 +++++++++-
>>>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>>>> b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>>>> index 9a45d840efeb..fa0e680a4b91 100644
>>>> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>>>> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
>>>> @@ -1067,8 +1067,16 @@ static void rcar_gen3_phy_usb2_remove(struct platform_device *pdev)  {
>>>>  	struct rcar_gen3_chan *channel = platform_get_drvdata(pdev);
>>>>
>>>> -	if (channel->is_otg_channel)
>>>> +	if (channel->is_otg_channel) {
>>>> +		/* Disable OTG interrupts so the IRQ handler cannot
>>>> +		 * schedule new work.
>>>> +		 */
>>>> +		rcar_gen3_control_otg_irq(channel, 0);
>>>> +
>>>>  		device_remove_file(&pdev->dev, &dev_attr_role);
>>>> +
>>>> +		cancel_work_sync(&channel->work);
>>>
>>> What about pending wq that is still about execute after
>>> "device_remove_file(&pdev->dev, &dev_attr_role);" ?
>> Hi Biju,
>>   I don't understand what you mean. cancel_work_sync is exactly what catches this kind of pending work —
>> it either cancels the work that hasn't run yet, or waits for the one that is currently running to finish,
>> and it is placed right.
>>  Could you explain it in more detail?
> 
> Assume you removed the file, and before cancel_work_sync(), the WQ get scheduled
> will it result in UAF bug mentioned in the commit message.
> 
The rcar_gen3_phy_usb2_work only uses chan. If you call
device_remove_file(&pdev->dev, &dev_attr_role) before
cancel_work_sync(&channel->work), it does not lead to a UAF.

On the contrary, if you call cancel_work_sync(&channel->work) first, the
sysfs node has not been removed yet. When the sysfs node is written to
(via store) again, it will schedule the work once more, rendering the
cancel_work_sync call ineffective.
> Maybe??
> 
> rcar_gen3_control_otg_irq(channel, 0);
> cancel_work_sync(&channel->work);
> device_remove_file(&pdev->dev, &dev_attr_role);
> 
> Cheers,
> Biju
> 
> 
> 
> 
>>
>> Thanks！
>> Pei.
>>
>>   > after device_remove_file.
>>
>>> Cheers,
>>> Biju
>>>
>>>> +	}
>>>>  }
>>>>
>>>>  static int rcar_gen3_phy_usb2_suspend(struct device *dev)
>>>> --
>>>> 2.25.1
>>>>
> 


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

RE: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition

[Biju Das]

> -----Original Message-----
> From: Pei Xiao <xiaopei01@kylinos.cn>
> Sent: 04 August 2026 10:31
> Subject: Re: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to
> race condition
> 
> 
> 
> 在 2026/8/4 17:08, Biju Das 写道:
> >
> >
> >> -----Original Message-----
> >> From: Pei Xiao <xiaopei01@kylinos.cn>
> >> Sent: 04 August 2026 10:05
> >> To: Biju Das <biju.das.jz@bp.renesas.com>; Yoshihiro Shimoda
> >> <yoshihiro.shimoda.uh@renesas.com>;
> >> vkoul@kernel.org; neil.armstrong@linaro.org; geert+renesas@glider.be;
> >> magnus.damm <magnus.damm@gmail.com>;
> >> linux-renesas-soc@vger.kernel.org; linux-phy@lists.infradead.org;
> >> linux- kernel@vger.kernel.org
> >> Subject: Re: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free
> >> in rcar_gen3_phy_usb2_remove due to race condition
> >>
> >>
> >>
> >> 在 2026/8/4 16:34, Biju Das 写道:
> >>> Hi Pei Xiao,
> >>>
> >>> Thanks for the patch.
> >>>
> >>>> -----Original Message-----
> >>>> From: Pei Xiao <xiaopei01@kylinos.cn>
> >>>> Sent: 04 August 2026 09:02
> >>>> Subject: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free
> >>>> in rcar_gen3_phy_usb2_remove due to race condition
> >>>>
> >>>> In rcar_gen3_phy_usb2_probe, &channel->work is bound with
> >>>> rcar_gen3_phy_usb2_work. rcar_gen3_phy_usb2_irq can schedule this
> >>>> work on system_wq via rcar_gen3_device_recognition(), and the role
> >>>> sysfs store can also schedule it
> >> via rcar_gen3_init_for_host() / rcar_gen3_init_for_peri().
> >>>>
> >>>> If we remove the device, rcar_gen3_phy_usb2_remove makes cleanup
> >>>> and the memory allocated for channel with devm_kzalloc() is
> >>>> released by the devm cleanup after the remove callback returns,
> >>>> while the work mentioned above may still be pending or running. The
> >>>> sequence of operations that may lead to a UAF bug
> >> is as follows:
> >>>>
> >>>> CPU0                                      CPU1
> >>>>
> >>>>                                           | rcar_gen3_phy_usb2_irq
> >>>>                                           | rcar_gen3_device_recognition
> >>>>                                           | rcar_gen3_init_for_host
> >>>>                                           | schedule_work(&ch->work)
> >>>> rcar_gen3_phy_usb2_remove                 |
> >>>> device_remove_file(&pdev->dev,            |
> >>>>                    &dev_attr_role)        |
> >>>> // remove returns                         |
> >>>> // devm cleanup: free_irq,                |
> >>>> // kfree(channel)                         |
> >>>>                                           | rcar_gen3_phy_usb2_work
> >>>>                                           | // use ch
> >>>> (use-after-free)
> >>>>
> >>>> Fix it by disabling the OTG interrupts, so the IRQ handler cannot
> >>>> schedule new work, and canceling the work before the remaining
> >>>> cleanup in rcar_gen3_phy_usb2_remove
> >> and the devm release of channel.
> >>>>
> >>>> Fixes: c14f8a4032ef ("phy: rcar-gen3-usb2: fix mutex_lock calling
> >>>> in
> >>>> interrupt")
> >>>> Assisted-by: Codex:deepseek-v4-flash
> >>>> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
> >>>> ---
> >>>>  drivers/phy/renesas/phy-rcar-gen3-usb2.c | 10 +++++++++-
> >>>>  1 file changed, 9 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> >>>> b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> >>>> index 9a45d840efeb..fa0e680a4b91 100644
> >>>> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> >>>> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> >>>> @@ -1067,8 +1067,16 @@ static void rcar_gen3_phy_usb2_remove(struct platform_device *pdev)  {
> >>>>  	struct rcar_gen3_chan *channel = platform_get_drvdata(pdev);
> >>>>
> >>>> -	if (channel->is_otg_channel)
> >>>> +	if (channel->is_otg_channel) {
> >>>> +		/* Disable OTG interrupts so the IRQ handler cannot
> >>>> +		 * schedule new work.
> >>>> +		 */
> >>>> +		rcar_gen3_control_otg_irq(channel, 0);
> >>>> +
> >>>>  		device_remove_file(&pdev->dev, &dev_attr_role);
> >>>> +
> >>>> +		cancel_work_sync(&channel->work);
> >>>
> >>> What about pending wq that is still about execute after
> >>> "device_remove_file(&pdev->dev, &dev_attr_role);" ?
> >> Hi Biju,
> >>   I don't understand what you mean. cancel_work_sync is exactly what
> >> catches this kind of pending work — it either cancels the work that
> >> hasn't run yet, or waits for the one that is currently running to finish, and it is placed right.
> >>  Could you explain it in more detail?
> >
> > Assume you removed the file, and before cancel_work_sync(), the WQ get
> > scheduled will it result in UAF bug mentioned in the commit message.
> >
> The rcar_gen3_phy_usb2_work only uses chan. If you call device_remove_file(&pdev->dev, &dev_attr_role)
> before cancel_work_sync(&channel->work), it does not lead to a UAF.
> 
> On the contrary, if you call cancel_work_sync(&channel->work) first, the sysfs node has not been removed
> yet. When the sysfs node is written to (via store) again, it will schedule the work once more, rendering
> the cancel_work_sync call ineffective.

Ok, I missed this.

Thanks for explanation.

Cheers,
Biju

> > Maybe??
> >
> > rcar_gen3_control_otg_irq(channel, 0);
> > cancel_work_sync(&channel->work); device_remove_file(&pdev->dev,
> > &dev_attr_role);
> >
> > Cheers,
> > Biju
> >
> >
> >
> >
> >>
> >> Thanks！
> >> Pei.
> >>
> >>   > after device_remove_file.
> >>
> >>> Cheers,
> >>> Biju
> >>>
> >>>> +	}
> >>>>  }
> >>>>
> >>>>  static int rcar_gen3_phy_usb2_suspend(struct device *dev)
> >>>> --
> >>>> 2.25.1
> >>>>
> >

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy