mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition
@ 2026-08-04  8:01 Pei Xiao
  2026-08-04  8:34 ` Biju Das
  0 siblings, 1 reply; 6+ messages in thread
From: Pei Xiao @ 2026-08-04  8:01 UTC (permalink / raw)
  To: yoshihiro.shimoda.uh, vkoul, neil.armstrong, geert+renesas,
	magnus.damm, linux-renesas-soc, linux-phy, linux-kernel
  Cc: 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition
  2026-08-04  8:01 [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition Pei Xiao
@ 2026-08-04  8:34 ` Biju Das
  2026-08-04  9:05   ` Pei Xiao
  0 siblings, 1 reply; 6+ messages in thread
From: Biju Das @ 2026-08-04  8:34 UTC (permalink / raw)
  To: Pei Xiao, Yoshihiro Shimoda, vkoul, neil.armstrong,
	geert+renesas, magnus.damm, linux-renesas-soc, linux-phy,
	linux-kernel

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
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition
  2026-08-04  8:34 ` Biju Das
@ 2026-08-04  9:05   ` Pei Xiao
  2026-08-04  9:08     ` Biju Das
  0 siblings, 1 reply; 6+ messages in thread
From: Pei Xiao @ 2026-08-04  9:05 UTC (permalink / raw)
  To: Biju Das, Yoshihiro Shimoda, vkoul, neil.armstrong,
	geert+renesas, magnus.damm, linux-renesas-soc, linux-phy,
	linux-kernel



在 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
>>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition
  2026-08-04  9:05   ` Pei Xiao
@ 2026-08-04  9:08     ` Biju Das
  2026-08-04  9:30       ` Pei Xiao
  0 siblings, 1 reply; 6+ messages in thread
From: Biju Das @ 2026-08-04  9:08 UTC (permalink / raw)
  To: Pei Xiao, Yoshihiro Shimoda, vkoul, neil.armstrong,
	geert+renesas, magnus.damm, linux-renesas-soc, linux-phy,
	linux-kernel



> -----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
> >>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition
  2026-08-04  9:08     ` Biju Das
@ 2026-08-04  9:30       ` Pei Xiao
  2026-08-04  9:38         ` Biju Das
  0 siblings, 1 reply; 6+ messages in thread
From: Pei Xiao @ 2026-08-04  9:30 UTC (permalink / raw)
  To: Biju Das, Yoshihiro Shimoda, vkoul, neil.armstrong,
	geert+renesas, magnus.damm, linux-renesas-soc, linux-phy,
	linux-kernel



在 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
>>>>
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition
  2026-08-04  9:30       ` Pei Xiao
@ 2026-08-04  9:38         ` Biju Das
  0 siblings, 0 replies; 6+ messages in thread
From: Biju Das @ 2026-08-04  9:38 UTC (permalink / raw)
  To: Pei Xiao, Yoshihiro Shimoda, vkoul, neil.armstrong,
	geert+renesas, magnus.damm, linux-renesas-soc, linux-phy,
	linux-kernel



> -----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
> >>>>
> >


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-04  9:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-04  8:01 [PATCH] phy: renesas: rcar-gen3-usb2: Fix use-after-free in rcar_gen3_phy_usb2_remove due to race condition Pei Xiao
2026-08-04  8:34 ` Biju Das
2026-08-04  9:05   ` Pei Xiao
2026-08-04  9:08     ` Biju Das
2026-08-04  9:30       ` Pei Xiao
2026-08-04  9:38         ` Biju Das

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®