| Message ID | 20250107020714.662708-1-make24@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,ARM] fix reference leak in locomo_init_one_child() | expand |
On Tue, Jan 07, 2025 at 10:07:14AM +0800, Ma Ke wrote: > Once device_register() failed, we should call put_device() to > decrement reference count for cleanup. Or it could cause memory leak. > > device_register() includes device_add(). 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: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Ma Ke <make24@iscas.ac.cn> > --- > Changes in v3: > - modified the patch as suggestions; > Changes in v2: > - modified the patch as suggestions. > --- > arch/arm/common/locomo.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c > index cb6ef449b987..9e275b2105c2 100644 > --- a/arch/arm/common/locomo.c > +++ b/arch/arm/common/locomo.c > @@ -220,13 +220,11 @@ static int > locomo_init_one_child(struct locomo *lchip, struct locomo_dev_info *info) > { > struct locomo_dev *dev; > - int ret; > + int ret = 0; The code around "ret" becomes: int ret = 0; ... ret = device_register(&dev->dev); Nothing between these two statements references "ret", and the present goto is eliminated in your patch. So, why do we need to initialise ret to zero where it is declared?
diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c index cb6ef449b987..9e275b2105c2 100644 --- a/arch/arm/common/locomo.c +++ b/arch/arm/common/locomo.c @@ -220,13 +220,11 @@ static int locomo_init_one_child(struct locomo *lchip, struct locomo_dev_info *info) { struct locomo_dev *dev; - int ret; + int ret = 0; dev = kzalloc(sizeof(struct locomo_dev), GFP_KERNEL); - if (!dev) { - ret = -ENOMEM; - goto out; - } + if (!dev) + return -ENOMEM; /* * If the parent device has a DMA mask associated with it, @@ -254,10 +252,9 @@ locomo_init_one_child(struct locomo *lchip, struct locomo_dev_info *info) NO_IRQ : lchip->irq_base + info->irq[0]; ret = device_register(&dev->dev); - if (ret) { - out: - kfree(dev); - } + if (ret) + put_device(&dev->dev); + return ret; }
Once device_register() failed, we should call put_device() to decrement reference count for cleanup. Or it could cause memory leak. device_register() includes device_add(). 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: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Ma Ke <make24@iscas.ac.cn> --- Changes in v3: - modified the patch as suggestions; Changes in v2: - modified the patch as suggestions. --- arch/arm/common/locomo.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)