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

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

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

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@vger.kernel.org
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
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;
+
 		ret = devm_request_irq(dev, irq, rcar_gen3_phy_usb2_irq,
 				       IRQF_SHARED, dev_name(dev), channel);
 		if (ret < 0)
-- 
2.43.0

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@gmail.com>
> 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@gmail.com>
> 
> 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@vger.kernel.org
> Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
> ---
> 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
> 

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

[khj]

 Hi Biju,

 Thanks for the review :)

 devm_add_action_or_reset() runs the action once immediately if it
fails to register it, and then returns the error. So on failure
rcar_gen3_phy_usb2_cancel_work() is called right away, i.e.
cancel_work_sync(&channel->work).

 That is safe here: channel->work has already been INIT_WORK()'d on
the line above, and the IRQ that schedules it (rcar_gen3_phy_usb2_irq)
is only requested after this call succeeds, so at that point the work
has never been queued. cancel_work_sync() on an
initialised-but-never-queued work is a no-op -- it just returns false.
(Unlike a timer, a work_struct has no uninitialised-object check on
the cancel path once INIT_WORK() has run.)

 So both paths are fine: either devres registers the cancel and runs
it on unbind, or registration fails, the cancel runs once immediately
(a harmless no-op), and probe returns the error.

 Thanks,
 Hyunjung

2026년 11월 9일 (월) 오후 8:45, Biju Das <biju.das.jz@bp.renesas.com>님이 작성:
>
> Hi hyunjungg,
>
> Thanks for the patch.
>
> > -----Original Message-----
> > From: hyunjungg <hj351016@gmail.com>
> > 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@gmail.com>
> >
> > 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@vger.kernel.org
> > Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
> > ---
> > 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
> >
>