[PATCH] scsi: fcoe: fix possible name leak when device_register() fails

[PATCH] scsi: fcoe: fix possible name leak when device_register() fails

[Yang Yingliang]

If device_register() returns error, the name allocated by
dev_set_name() need be freed. As comment of device_register()
says, it should use put_device() to give up the reference in
the error path. So fix this by calling put_device(), then the
name can be freed in kobject_cleanup().

The 'fcf' is freed in fcoe_fcf_device_release(), so the kfree()
in the error path can be removed.

The 'ctlr' is freed in fcoe_ctlr_device_release(), so don't use
the error label, just return NULL after calling put_device().

Fixes: 9a74e884ee71 ("[SCSI] libfcoe: Add fcoe_sysfs")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/scsi/fcoe/fcoe_sysfs.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c
index af658aa38fed..6260aa5ea6af 100644
--- a/drivers/scsi/fcoe/fcoe_sysfs.c
+++ b/drivers/scsi/fcoe/fcoe_sysfs.c
@@ -830,14 +830,15 @@ struct fcoe_ctlr_device *fcoe_ctlr_device_add(struct device *parent,
 
 	dev_set_name(&ctlr->dev, "ctlr_%d", ctlr->id);
 	error = device_register(&ctlr->dev);
-	if (error)
-		goto out_del_q2;
+	if (error) {
+		destroy_workqueue(ctlr->devloss_work_q);
+		destroy_workqueue(ctlr->work_q);
+		put_device(&ctlr->dev);
+		return NULL;
+	}
 
 	return ctlr;
 
-out_del_q2:
-	destroy_workqueue(ctlr->devloss_work_q);
-	ctlr->devloss_work_q = NULL;
 out_del_q:
 	destroy_workqueue(ctlr->work_q);
 	ctlr->work_q = NULL;
@@ -1036,16 +1037,16 @@ struct fcoe_fcf_device *fcoe_fcf_device_add(struct fcoe_ctlr_device *ctlr,
 	fcf->selected = new_fcf->selected;
 
 	error = device_register(&fcf->dev);
-	if (error)
-		goto out_del;
+	if (error) {
+		put_device(&fcf->dev);
+		goto out;
+	}
 
 	fcf->state = FCOE_FCF_STATE_CONNECTED;
 	list_add_tail(&fcf->peers, &ctlr->fcfs);
 
 	return fcf;
 
-out_del:
-	kfree(fcf);
 out:
 	return NULL;
 }
-- 
2.25.1

Re: [PATCH] scsi: fcoe: fix possible name leak when device_register() fails

[Martin K. Petersen]

Yang,

> If device_register() returns error, the name allocated by
> dev_set_name() need be freed. As comment of device_register() says, it
> should use put_device() to give up the reference in the error path. So
> fix this by calling put_device(), then the name can be freed in
> kobject_cleanup().

Applied to 6.2/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: fcoe: fix possible name leak when device_register() fails

[Martin K. Petersen]

On Sat, 12 Nov 2022 17:43:10 +0800, Yang Yingliang wrote:

> If device_register() returns error, the name allocated by
> dev_set_name() need be freed. As comment of device_register()
> says, it should use put_device() to give up the reference in
> the error path. So fix this by calling put_device(), then the
> name can be freed in kobject_cleanup().
> 
> The 'fcf' is freed in fcoe_fcf_device_release(), so the kfree()
> in the error path can be removed.
> 
> [...]

Applied to 6.2/scsi-queue, thanks!

[1/1] scsi: fcoe: fix possible name leak when device_register() fails
      https://git.kernel.org/mkp/scsi/c/47b6a122c7b6

-- 
Martin K. Petersen	Oracle Linux Engineering