A recent change fixing a device reference leak in a UDC driver
introduced a potential use-after-free in the non-OF case as the
isp1301_get_client() helper only increases the reference count for the
returned I2C device in the OF case.

Increment the reference count also for non-OF so that the caller can
decrement it unconditionally.

Note that this is inherently racy just as using the returned I2C device
is since nothing is preventing the PHY driver from being unbound while
in use.

Fixes: c84117912bdd ("USB: lpc32xx_udc: Fix error handling in probe")
Cc: stable@vger.kernel.org
Cc: Ma Ke <make24@iscas.ac.cn>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/phy/phy-isp1301.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-isp1301.c b/drivers/usb/phy/phy-isp1301.c
index f9b5c411aee4..2940f0c84e1b 100644
--- a/drivers/usb/phy/phy-isp1301.c
+++ b/drivers/usb/phy/phy-isp1301.c
@@ -149,7 +149,12 @@ struct i2c_client *isp1301_get_client(struct device_node *node)
 		return client;
 
 	/* non-DT: only one ISP1301 chip supported */
-	return isp1301_i2c_client;
+	if (isp1301_i2c_client) {
+		get_device(&isp1301_i2c_client->dev);
+		return isp1301_i2c_client;
+	}
+
+	return NULL;
 }
 EXPORT_SYMBOL_GPL(isp1301_get_client);
 
-- 
2.51.2

On 12/18/25 17:35, Johan Hovold wrote:
> A recent change fixing a device reference leak in a UDC driver
> introduced a potential use-after-free in the non-OF case as the
> isp1301_get_client() helper only increases the reference count for the
> returned I2C device in the OF case.

Fortunatly there is no non-OF users of this driver, it's been discussed
recently.

> 
> Increment the reference count also for non-OF so that the caller can
> decrement it unconditionally.
> 
> Note that this is inherently racy just as using the returned I2C device
> is since nothing is preventing the PHY driver from being unbound while
> in use.
> 
> Fixes: c84117912bdd ("USB: lpc32xx_udc: Fix error handling in probe")
> Cc: stable@vger.kernel.org
> Cc: Ma Ke <make24@iscas.ac.cn>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>   drivers/usb/phy/phy-isp1301.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/phy/phy-isp1301.c b/drivers/usb/phy/phy-isp1301.c
> index f9b5c411aee4..2940f0c84e1b 100644
> --- a/drivers/usb/phy/phy-isp1301.c
> +++ b/drivers/usb/phy/phy-isp1301.c
> @@ -149,7 +149,12 @@ struct i2c_client *isp1301_get_client(struct device_node *node)
>   		return client;
>   
>   	/* non-DT: only one ISP1301 chip supported */
> -	return isp1301_i2c_client;
> +	if (isp1301_i2c_client) {
> +		get_device(&isp1301_i2c_client->dev);
> +		return isp1301_i2c_client;
> +	}
> +
> +	return NULL;
>   }
>   EXPORT_SYMBOL_GPL(isp1301_get_client);
>   

Okay, let's go the way of fixing the broken commit instead of its reversal.

Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>

-- 
Best wishes,
Vladimir

On Fri, Dec 19, 2025 at 02:15:12AM +0200, Vladimir Zapolskiy wrote:
> On 12/18/25 17:35, Johan Hovold wrote:
> > A recent change fixing a device reference leak in a UDC driver
> > introduced a potential use-after-free in the non-OF case as the
> > isp1301_get_client() helper only increases the reference count for the
> > returned I2C device in the OF case.
> 
> Fortunatly there is no non-OF users of this driver, it's been discussed
> recently.

Yeah, I saw the discussion, but figured it was best to just fix up the
existing code before you guys get on with ripping out the legacy
support.

> > Increment the reference count also for non-OF so that the caller can
> > decrement it unconditionally.
> > 
> > Note that this is inherently racy just as using the returned I2C device
> > is since nothing is preventing the PHY driver from being unbound while
> > in use.
> > 
> > Fixes: c84117912bdd ("USB: lpc32xx_udc: Fix error handling in probe")
> > Cc: stable@vger.kernel.org
> > Cc: Ma Ke <make24@iscas.ac.cn>
> > Signed-off-by: Johan Hovold <johan@kernel.org>

> > @@ -149,7 +149,12 @@ struct i2c_client *isp1301_get_client(struct device_node *node)
> >   		return client;
> >   
> >   	/* non-DT: only one ISP1301 chip supported */
> > -	return isp1301_i2c_client;
> > +	if (isp1301_i2c_client) {
> > +		get_device(&isp1301_i2c_client->dev);
> > +		return isp1301_i2c_client;
> > +	}
> > +
> > +	return NULL;
> >   }
> >   EXPORT_SYMBOL_GPL(isp1301_get_client);
> >   
> 
> Okay, let's go the way of fixing the broken commit instead of its reversal.
> 
> Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>

Thanks for reviewing.

Johan