RE: [PATCH v2] phy: rcar-gen3-usb2: cancel OTG work on unbind to avoid use-after-free

[Biju Das]

Hi hyunjungg,

Thanks for the patch.

> -----Original Message-----
> From: hyunjungg <hj351016@xxxxxxxxx>
> Sent: 09 November 2026 11:31
> Subject: [PATCH v2] phy: rcar-gen3-usb2: cancel OTG work on unbind to avoid use-after-free
> 
> From: Hyunjung Ko <hj351016@xxxxxxxxx>
> 
> The OTG role/VBUS interrupt (rcar_gen3_phy_usb2_irq) and the role sysfs attribute schedule channel-
> >work (rcar_gen3_phy_usb2_work), whose handler dereferences the channel (ch->extcon). The channel is
> devm-allocated, so it is freed at unbind, but nothing cancels channel->work:
> rcar_gen3_phy_usb2_remove() only drops the role sysfs file. A work scheduled just before unbind can
> run after the channel has been freed, a use-after-free.
> 
> Cancel the work via devm, registered before the interrupt is requested so that on unbind devm frees
> the interrupt first (no new work can be
> scheduled) and then runs the cancel action before the channel is freed.
> 
> Fixes: c14f8a4032ef ("phy: rcar-gen3-usb2: fix mutex_lock calling in interrupt")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Hyunjung Ko <hj351016@xxxxxxxxx>
> ---
> Changes in v2 (thanks to the Sashiko AI review):
>  - correct the Fixes: tag to c14f8a4032ef, which introduced channel->work
>    and rcar_gen3_phy_usb2_work(), rather than 2b38543c8db1 which only added
>    synchronous extcon handling
>  drivers/phy/renesas/phy-rcar-gen3-usb2.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/phy/renesas/phy-rcar-gen3-usb2.c b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> index b5ba751..35e519a 100644
> --- a/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> +++ b/drivers/phy/renesas/phy-rcar-gen3-usb2.c
> @@ -1171,6 +1171,13 @@ static int rcar_gen3_phy_usb2_vbus_regulator_register(struct rcar_gen3_chan
> *cha
>  	return rcar_gen3_phy_usb2_vbus_regulator_get_exclusive_enable(channel, enable);  }
> 
> +static void rcar_gen3_phy_usb2_cancel_work(void *data) {
> +	struct rcar_gen3_chan *channel = data;
> +
> +	cancel_work_sync(&channel->work);
> +}
> +
>  static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)  {
>  	struct device *dev = &pdev->dev;
> @@ -1277,6 +1284,17 @@ static int rcar_gen3_phy_usb2_probe(struct platform_device *pdev)
>  		return irq;
>  	} else if (irq > 0) {
>  		INIT_WORK(&channel->work, rcar_gen3_phy_usb2_work);
> +		/*
> +		 * Registered before the IRQ so that, on unbind, devm frees the
> +		 * IRQ first (no more work can be scheduled) and then cancels any
> +		 * work still pending before the channel is freed.
> +		 */
> +		ret = devm_add_action_or_reset(dev,
> +					       rcar_gen3_phy_usb2_cancel_work,
> +					       channel);
> +		if (ret < 0)
> +			return ret;

I have a question,

What happens if this return error? Is it ok to call
cancel_work_sync() before scheduling it?

Cheers,
Biju

> +
>  		ret = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
>  				       IRQF_SHARED, dev_name(dev), channel);
>  		if (ret < 0)
> --
> 2.43.0
>