Message ID | 20250107085129.812835-1-make24@iscas.ac.cn |
---|---|
State | New |
Headers | show |
Series | phy: Fix error handling in tegra_xusb_port_init | expand |
On Tue, Jan 07, 2025 at 04:51:29PM +0800, Ma Ke wrote: > The reference count of the device incremented in device_initialize() > is not decremented properly when device_add() fails. Change > device_unregister() to a put_device() call before returning from the > function to decrement reference count for cleanup. Or it could cause > memory leak. > > As comment of device_add() says, 'if device_add() succeeds, you should > call device_del() when you want to get rid of it. If device_add() has > not succeeded, use only put_device() to drop the reference count'. > > Found by code review. While the patch looks correct, the commit message seems confusing to me. device_unregister() will also call put_device() after first calling device_del(). So the reference count /is/ properly balanced. Instead, the kerneldoc comment for device_add() says this: * NOTE: _Never_ directly free @dev after calling this function, even * if it returned an error! Always use put_device() to give up your * reference instead. * * Rule of thumb is: if device_add() succeeds, you should call * device_del() when you want to get rid of it. If device_add() has * *not* succeeded, use *only* put_device() to drop the reference * count. So the real reason that this patch is correct is because device_unregister() should only be called after device_add() succeeded because device_del() undoes what device_add() does if successful. In this case we only need to undo what device_initialize() does, and that is what put_device() will do. Thierry > > Cc: stable@vger.kernel.org > Fixes: 53d2a715c240 ("phy: Add Tegra XUSB pad controller support") > Signed-off-by: Ma Ke <make24@iscas.ac.cn> > --- > drivers/phy/tegra/xusb.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c > index 79d4814d758d..c89df95aa6ca 100644 > --- a/drivers/phy/tegra/xusb.c > +++ b/drivers/phy/tegra/xusb.c > @@ -548,16 +548,16 @@ static int tegra_xusb_port_init(struct tegra_xusb_port *port, > > err = dev_set_name(&port->dev, "%s-%u", name, index); > if (err < 0) > - goto unregister; > + goto put_device; > > err = device_add(&port->dev); > if (err < 0) > - goto unregister; > + goto put_device; > > return 0; > > -unregister: > - device_unregister(&port->dev); > +put_device: > + put_device(&port->dev); > return err; > } > > -- > 2.25.1 >
diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c index 79d4814d758d..c89df95aa6ca 100644 --- a/drivers/phy/tegra/xusb.c +++ b/drivers/phy/tegra/xusb.c @@ -548,16 +548,16 @@ static int tegra_xusb_port_init(struct tegra_xusb_port *port, err = dev_set_name(&port->dev, "%s-%u", name, index); if (err < 0) - goto unregister; + goto put_device; err = device_add(&port->dev); if (err < 0) - goto unregister; + goto put_device; return 0; -unregister: - device_unregister(&port->dev); +put_device: + put_device(&port->dev); return err; }
The reference count of the device incremented in device_initialize() is not decremented properly when device_add() fails. Change device_unregister() to a put_device() call before returning from the function to decrement reference count for cleanup. Or it could cause memory leak. As comment of device_add() says, 'if device_add() succeeds, you should call device_del() when you want to get rid of it. If device_add() has not succeeded, use only put_device() to drop the reference count'. Found by code review. Cc: stable@vger.kernel.org Fixes: 53d2a715c240 ("phy: Add Tegra XUSB pad controller support") Signed-off-by: Ma Ke <make24@iscas.ac.cn> --- drivers/phy/tegra/xusb.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)