[PATCH] EDAC/device: Fix double free on sysfs registration failure

Guangshuo Li posted 1 patch 1 week, 4 days ago
drivers/edac/edac_device.c       |  2 +-
drivers/edac/edac_device_sysfs.c | 20 +++++++++++++-------
2 files changed, 14 insertions(+), 8 deletions(-)
[PATCH] EDAC/device: Fix double free on sysfs registration failure
Posted by Guangshuo Li 1 week, 4 days ago
edac_device_alloc_ctl_info() allocates dev_ctl and passes it to
edac_device_register_sysfs_main_kobj() to initialize its embedded
kobject.

If kobject_init_and_add() fails, the helper calls kobject_put(). The
final reference invokes edac_device_ctrl_master_release(), which calls
__edac_device_free_ctl_info() and frees dev_ctl for the first time.

After the helper returns an error, edac_device_alloc_ctl_info() jumps
to its free label and calls __edac_device_free_ctl_info() on the same
object again, resulting in a double free. The helper also accesses
edac_dev after kobject_put() may already have freed it.

Make the registration helper release edac_dev on every failure path
and return directly from the allocator when registration fails. Avoid
accessing the object after kobject_put() transfers cleanup to the
release callback.

This issue was found by a static analysis tool I am developing.

Fixes: 17ed808ad243 ("EDAC: Fix reference count leaks")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/edac/edac_device.c       |  2 +-
 drivers/edac/edac_device_sysfs.c | 20 +++++++++++++-------
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/edac/edac_device.c b/drivers/edac/edac_device.c
index cf0d3c2dfc04..ce986dc341cb 100644
--- a/drivers/edac/edac_device.c
+++ b/drivers/edac/edac_device.c
@@ -133,7 +133,7 @@ edac_device_alloc_ctl_info(unsigned pvt_sz, char *dev_name, unsigned nr_instance
 	 */
 	err = edac_device_register_sysfs_main_kobj(dev_ctl);
 	if (err)
-		goto free;
+		return NULL;
 
 	/* at this point, the root kobj is valid, and in order to
 	 * 'free' the object, then the function:
diff --git a/drivers/edac/edac_device_sysfs.c b/drivers/edac/edac_device_sysfs.c
index b1c2717cd023..acab6c07f304 100644
--- a/drivers/edac/edac_device_sysfs.c
+++ b/drivers/edac/edac_device_sysfs.c
@@ -247,15 +247,17 @@ int edac_device_register_sysfs_main_kobj(struct edac_device_ctl_info *edac_dev)
 	edac_dev->owner = THIS_MODULE;
 
 	if (!try_module_get(edac_dev->owner))
-		goto err_out;
+		goto free_ctl_info;
 
 	/* register */
 	dev_root = bus_get_dev_root(edac_subsys);
-	if (dev_root) {
-		err = kobject_init_and_add(&edac_dev->kobj, &ktype_device_ctrl,
-					   &dev_root->kobj, "%s", edac_dev->name);
-		put_device(dev_root);
-	}
+	if (!dev_root)
+		goto module_put;
+
+	err = kobject_init_and_add(&edac_dev->kobj, &ktype_device_ctrl,
+				   &dev_root->kobj, "%s", edac_dev->name);
+	put_device(dev_root);
+
 	if (err) {
 		edac_dbg(1, "Failed to register '.../edac/%s'\n",
 			 edac_dev->name);
@@ -274,9 +276,13 @@ int edac_device_register_sysfs_main_kobj(struct edac_device_ctl_info *edac_dev)
 	/* Error exit stack */
 err_kobj_reg:
 	kobject_put(&edac_dev->kobj);
+	return err;
+
+module_put:
 	module_put(edac_dev->owner);
 
-err_out:
+free_ctl_info:
+	__edac_device_free_ctl_info(edac_dev);
 	return err;
 }
 
-- 
2.43.0