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: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 <[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.
> >
> 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
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy