On 8/19/23 01:03, Zhu Wang wrote:
> Previous commit 04b5b5cb0136 ("scsi: core: Fix possible memory leak if
> device_add() fails") introduced a double free when device_add() failed.
> When device_add() failed, the put_device(&rc->dev) is called, which will
> call raid_component_release(), and this function will call
> put_device(rc->dev.parent) and kfree(rc), but in the error path of
> raid_component_release() above two functions are called again, so 'rc' are
> freed twice.
>
> We do not just revert the patch, since the memory allocated by
> dev_set_name() is not released in the error path, it should be released
> through calling put_device(&rc->dev) which will call kobject_cleanup(),
> and the device name will be released in it. Though the commit 04b5b5cb0136
> ("scsi: core: Fix possible memory leak if device_add() fails") fixed the
> memory leak, it didn't consider the double free problem. We removed
> put_device(rc->dev.parent) and kfree(rc) in the error path, and we moved
> put_device(&rc->dev) after rd->component-count--, since after
> put_device(&rc->dev) is called, 'rc' is freed, so list_del(&rc->node)
> cannot be called again.
>
> Fixes: 04b5b5cb0136 ("scsi: core: Fix possible memory leak if device_add() fails")
> Signed-off-by: Zhu Wang <[email protected]>
> ---
> drivers/scsi/raid_class.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/raid_class.c b/drivers/scsi/raid_class.c
> index 711252e52d8e..86ed1f66d749 100644
> --- a/drivers/scsi/raid_class.c
> +++ b/drivers/scsi/raid_class.c
> @@ -248,11 +248,9 @@ int raid_component_add(struct raid_template *r,struct device *raid_dev,
> return 0;
>
> err_out:
> - put_device(&rc->dev);
> list_del(&rc->node);
> rd->component_count--;
> - put_device(component_dev);
> - kfree(rc);
> + put_device(&rc->dev);
> return err;
> }
> EXPORT_SYMBOL(raid_component_add);
Please consider replacing the above patch by the patch below:
[PATCH] scsi: Remove raid_component_add()
The raid_component_add() function was added to the kernel tree via
patch "[SCSI] embryonic RAID class" (2005). Remove this function since
it never has had any callers in the Linux kernel.
Signed-off-by: Bart Van Assche <[email protected]>
---
drivers/scsi/raid_class.c | 38 --------------------------------------
include/linux/raid_class.h | 4 ----
2 files changed, 42 deletions(-)
diff --git a/drivers/scsi/raid_class.c b/drivers/scsi/raid_class.c
index 898a0bdf8df6..99f806d1d317 100644
--- a/drivers/scsi/raid_class.c
+++ b/drivers/scsi/raid_class.c
@@ -218,44 +218,6 @@ static void raid_component_release(struct device *dev)
kfree(rc);
}
-int raid_component_add(struct raid_template *r,struct device *raid_dev,
- struct device *component_dev)
-{
- struct device *cdev =
- attribute_container_find_class_device(&r->raid_attrs.ac,
- raid_dev);
- struct raid_component *rc;
- struct raid_data *rd = dev_get_drvdata(cdev);
- int err;
-
- rc = kzalloc(sizeof(*rc), GFP_KERNEL);
- if (!rc)
- return -ENOMEM;
-
- INIT_LIST_HEAD(&rc->node);
- device_initialize(&rc->dev);
- rc->dev.release = raid_component_release;
- rc->dev.parent = get_device(component_dev);
- rc->num = rd->component_count++;
-
- dev_set_name(&rc->dev, "component-%d", rc->num);
- list_add_tail(&rc->node, &rd->component_list);
- rc->dev.class = &raid_class.class;
- err = device_add(&rc->dev);
- if (err)
- goto err_out;
-
- return 0;
-
-err_out:
- list_del(&rc->node);
- rd->component_count--;
- put_device(component_dev);
- kfree(rc);
- return err;
-}
-EXPORT_SYMBOL(raid_component_add);
-
struct raid_template *
raid_class_attach(struct raid_function_template *ft)
{
diff --git a/include/linux/raid_class.h b/include/linux/raid_class.h
index 6a9b177d5c41..e50416ba9cd9 100644
--- a/include/linux/raid_class.h
+++ b/include/linux/raid_class.h
@@ -77,7 +77,3 @@ DEFINE_RAID_ATTRIBUTE(enum raid_state, state)
struct raid_template *raid_class_attach(struct raid_function_template *);
void raid_class_release(struct raid_template *);
-
-int __must_check raid_component_add(struct raid_template *, struct device *,
- struct device *);
-