On 02/08/2023 04:10, Zhu Wang wrote:
The driver do not handle the possible returning error of dev_set_name,
if it returned fail, some operations should be rollback or there may be
possible memory leak.
What memory are we possibly leaking? Please explain how.
We use put_device to free the device and use kfree
to free the memory in the error handle path.
Much of the core driver code in drivers/base - where dev_set_name()
lives - does not check the return code. Why is SCSI special?
I'd say that the core driver code should be fixed up first in terms of
usage, and then the rest.
BTW, as I see, dev_set_name() only fails for a small memory alloc
failure. If memory alloc failure like this occurs, then things are not
going to work properly anyway. Just not having the device name set
should not make things worse.
Fixes: 71610f55fa4d ("[SCSI] struct device - replace bus_id with dev_name(), dev_set_name()")
Signed-off-by: Zhu Wang <wangzhu9@xxxxxxxxxx>
Reviewed-by: Bart Van Assche <bvanassche@xxxxxxx>
---
Changes in v2:
- Add put_device(parent) in the error path.
---
drivers/scsi/scsi_scan.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index aa13feb17c62..de7e503bfcab 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -509,7 +509,14 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
device_initialize(dev);
kref_init(&starget->reap_ref);
dev->parent = get_device(parent);
- dev_set_name(dev, "target%d:%d:%d", shost->host_no, channel, id);
+ error = dev_set_name(dev, "target%d:%d:%d", shost->host_no, channel, id);
+ if (error) {
+ dev_err(dev, "%s: dev_set_name failed, error %d\n", __func__, error);
Ironically dev_err() is used, but the dev name could not be set. And
this dev has no init_name. So how does the print look in practice?
+ put_device(parent);
+ put_device(dev);
+ kfree(starget);
+ return NULL;
+ }
dev->bus = &scsi_bus_type;
dev->type = &scsi_target_type;
scsi_enable_async_suspend(dev);
Thanks,
John
