linux-kernel - Re: [PATCH v2 2/5] usb: phy: isp1301: fix non-OF device reference imbalance

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aUTuxjyqh3EE_wJd@hovoldconsulting.com>
Date: Fri, 19 Dec 2025 07:20:54 +0100
From: Johan Hovold <johan@...nel.org>
To: Vladimir Zapolskiy <vz@...ia.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Piotr Wojtaszczyk <piotr.wojtaszczyk@...esys.com>,
	Alan Stern <stern@...land.harvard.edu>, Ma Ke <make24@...as.ac.cn>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH v2 2/5] usb: phy: isp1301: fix non-OF device reference
 imbalance

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@...r.kernel.org
> > Cc: Ma Ke <make24@...as.ac.cn>
> > Signed-off-by: Johan Hovold <johan@...nel.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@...ia.com>

Thanks for reviewing.

Johan