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

> -----Original Message-----
> From: Pei Xiao <[email protected]>
> Sent: 04 August 2026 10:05
> To: Biju Das <[email protected]>; Yoshihiro Shimoda <[email protected]>;
> [email protected]; [email protected]; [email protected]; magnus.damm
> <[email protected]>; [email protected]; [email protected]; linux-
> [email protected]
> 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 <[email protected]>
> >> 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 <[email protected]>
> >> ---
> >>  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
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy