s390: scsi: fix error path
Subsystems (3)
PATCH 1/14 s390: scsi: fix error path
zfcp_unit_add() doesn't free all allocated resources in error case.
---
I cannot compile this driver, so it is not tested at all.
drivers/s390/scsi/zfcp_unit.c | 28 +++++++++++++++++----------- 1 files changed, 17 insertions(+), 11 deletions(-)
drivers/s390/scsi/zfcp_unit.c +17 -11
@@ -119,6 +119,7 @@ static void zfcp_unit_release(struct device *dev)int zfcp_unit_add (struct zfcp_port *port,u64 fcp_lun) {struct zfcp_unit *unit; +int err; unit =zfcp_unit_find (port, fcp_lun);if (unit) {
@@ -136,21 +137,20 @@ int zfcp_unit_add(struct zfcp_port *port, u64 fcp_lun) unit->dev.release = zfcp_unit_release;INIT_WORK (&unit->scsi_work, zfcp_unit_scsi_scan_work); -if (dev_set_name (&unit->dev,"0x%016llx" , - (unsigned long long ) fcp_lun)) { -kfree (unit); -return -ENOMEM ; - } + err =dev_set_name (&unit->dev,"0x%016llx" , + (unsigned long long ) fcp_lun); +if (err) + goto err_free; -if (device_register (&unit->dev)) { + err =device_register (&unit->dev); +if (err) {put_device (&unit->dev); -return -ENOMEM ; + goto err_free; } -if (sysfs_create_group (&unit->dev.kobj, &zfcp_sysfs_unit_attrs)) { -device_unregister (&unit->dev); -return -EINVAL ; - } + err =sysfs_create_group (&unit->dev.kobj, &zfcp_sysfs_unit_attrs); +if (err) + goto err_unreg;get_device (&port->dev);
@@ -161,6 +161,12 @@ int zfcp_unit_add(struct zfcp_port *port, u64 fcp_lun)zfcp_unit_scsi_scan (unit);return 0 ; + +err_unreg: +device_unregister (&unit->dev); +err_free: +kfree (unit); +return err; }/**
Signature · 1 line
1.7.0.4
Quoted 3 lines · Vasiliy Kulikov
zfcp_unit_add()doesn't free all allocated resources in error case.Signed-off-by Vasiliy Kulikov <[email protected]>
Where do you see a problem? The memory management for the unit device has to be done the way the current code does:
After dev_set_name, the struct has to be deleted through put_device that calls the release functions and also frees the memory allocated for the device name.
When the memory is freed through the release function, an additional call to kfree is wrong, the release function already calls kfree. device_unregister calls the put_device, so this also frees the memory through the release function.
The only thing i found wrong is that zfcp_unit_release calls put_device on the port, and this could be done before get_device was called on the port. I will prepare a patch to move the get_device.
Quoted 70 lines
---
I cannot compile this driver, so it is not tested at all.drivers/s390/scsi/zfcp_unit.c | 28 +++++++++++++++++----------- 1 files changed, 17 insertions(+), 11 deletions(-)drivers/s390/scsi/zfcp_unit.c +17
-11@@ -119,6 +119,7 @@ static void zfcp_unit_release(struct device *dev)int zfcp_unit_add (struct zfcp_port *port,u64 fcp_lun) {struct zfcp_unit *unit; +int err; unit =zfcp_unit_find (port, fcp_lun);if (unit) {@@ -136,21 +137,20 @@ int zfcp_unit_add(struct zfcp_port *port, u64 fcp_lun) unit->dev.release = zfcp_unit_release;INIT_WORK (&unit->scsi_work, zfcp_unit_scsi_scan_work); -if (dev_set_name (&unit->dev,"0x%016llx" , - (unsigned long long ) fcp_lun)) { -kfree (unit); -return -ENOMEM ; - } + err =dev_set_name (&unit->dev,"0x%016llx" , + (unsigned long long ) fcp_lun); +if (err) + goto err_free; -if (device_register (&unit->dev)) { + err =device_register (&unit->dev); +if (err) {put_device (&unit->dev); -return -ENOMEM ; + goto err_free; } -if (sysfs_create_group (&unit->dev.kobj, &zfcp_sysfs_unit_attrs)) { -device_unregister (&unit->dev); -return -EINVAL ; - } + err =sysfs_create_group (&unit->dev.kobj, &zfcp_sysfs_unit_attrs); +if (err) + goto err_unreg;get_device (&port->dev);@@ -161,6 +161,12 @@ int zfcp_unit_add(struct zfcp_port *port, u64 fcp_lun)zfcp_unit_scsi_scan (unit);return 0 ; + +err_unreg: +device_unregister (&unit->dev); +err_free: +kfree (unit); +return err; }/** Signature · 6 lines
1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html