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

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
>=20
> In rcar_gen3_phy_usb2_probe, &channel->work is bound with rcar_gen3_phy_u=
sb2_work. rcar_gen3_phy_usb2_irq
> can schedule this work on system_wq via rcar_gen3_device_recognition(), a=
nd the role sysfs store can also
> schedule it via rcar_gen3_init_for_host() / rcar_gen3_init_for_peri().
>=20
> 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 call=
back returns, while the work
> mentioned above may still be pending or running. The sequence of operatio=
ns that may lead to a UAF bug is
> as follows:
>=20
> CPU0                                      CPU1
>=20
>                                           | 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)
>=20
> Fix it by disabling the OTG interrupts, so the IRQ handler cannot schedul=
e new work, and canceling the
> work before the remaining cleanup in rcar_gen3_phy_usb2_remove and the de=
vm release of channel.
>=20
> Fixes: c14f8a4032ef ("phy: rcar-gen3-usb2: fix mutex_lock calling in inte=
rrupt")
> 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(-)
>=20
> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renes=
as/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 platf=
orm_device *pdev)  {
>  	struct rcar_gen3_chan *channel =3D platform_get_drvdata(pdev);
>=20
> -	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

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