[PATCH v3] EDAC/sysfs: Fix kobject cleanup after kobject_init_and_add() failure

[Guangshuo Li]

If kobject_init_and_add() fails, the initialized kobject should be
released with kobject_put(). Otherwise the kobject may leak resources
associated with it.

Some EDAC sysfs error paths currently drop the parent kobject reference
directly after kobject_init_and_add() fails. However, the corresponding
release callbacks of the child kobjects already drop those parent
references. Call kobject_put() on the initialized child kobject instead,
so the release callbacks can unwind the references properly.

In edac_device_register_sysfs_main_kobj(), kobject_put() may call
edac_device_ctrl_master_release(), which drops the module reference and
frees the edac_device_ctl_info object. The error path then calls
module_put(edac_dev->owner). This dereferences edac_dev after it may have
been freed, causing a possible use-after-free, and also drops the module
reference twice.

Track whether kobject_init_and_add() has actually been called. If it has,
rely on the kobject release callback to drop the module reference;
otherwise, drop the module reference directly.

Also handle the EDAC PCI top-level kobject setup carefully: if
kobject_init_and_add() was called and failed, use kobject_put(); if it
was never called, free the allocated kobject directly.

These issues were found by a static analysis tool I am developing.

Fixes: 17ed808ad2431 ("EDAC: Fix reference count leaks")
Fixes: b2ed215a3338 ("Kobject: change drivers/edac to use kobject_init_and_add")
Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
---
v2:
  - Move kobj_initialized assignment to the kobject_init_and_add() call
    site so it records whether the kobject has actually been initialized.
v3:
  - Fix similar kobject_init_and_add() error paths under drivers/edac/.
  - Fold in the previous edac_device_create_instance() cleanup fix.
  - Put the initialized child kobject instead of the parent kobject.
  - Avoid calling kobject_put() on edac_pci_top_main_kobj if
    kobject_init_and_add() was not called.

 drivers/edac/edac_device_sysfs.c | 12 ++++++++----
 drivers/edac/edac_pci_sysfs.c    | 11 +++++++++--
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/edac/edac_device_sysfs.c b/drivers/edac/edac_device_sysfs.c
index fcebc4ffea26..5e6141a94716 100644
--- a/drivers/edac/edac_device_sysfs.c
+++ b/drivers/edac/edac_device_sysfs.c
@@ -231,6 +231,7 @@ int edac_device_register_sysfs_main_kobj(struct edac_device_ctl_info *edac_dev)
 	struct device *dev_root;
 	const struct bus_type *edac_subsys;
 	int err = -ENODEV;
+	bool kobj_initialized = false;
 
 	edac_dbg(1, "\n");
 
@@ -256,6 +257,7 @@ int edac_device_register_sysfs_main_kobj(struct edac_device_ctl_info *edac_dev)
 	if (dev_root) {
 		err = kobject_init_and_add(&edac_dev->kobj, &ktype_device_ctrl,
 					   &dev_root->kobj, "%s", edac_dev->name);
+		kobj_initialized = true;
 		put_device(dev_root);
 	}
 	if (err) {
@@ -275,8 +277,10 @@ 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);
-	module_put(edac_dev->owner);
+	if (kobj_initialized)
+		kobject_put(&edac_dev->kobj);
+	else
+		module_put(edac_dev->owner);
 
 err_out:
 	return err;
@@ -523,7 +527,7 @@ static int edac_device_create_block(struct edac_device_ctl_info *edac_dev,
 				   "%s", block->name);
 	if (err) {
 		edac_dbg(1, "Failed to register instance '%s'\n", block->name);
-		kobject_put(main_kobj);
+		kobject_put(&block->kobj);
 		err = -ENODEV;
 		goto err_out;
 	}
@@ -622,7 +626,7 @@ static int edac_device_create_instance(struct edac_device_ctl_info *edac_dev,
 	if (err != 0) {
 		edac_dbg(2, "Failed to register instance '%s'\n",
 			 instance->name);
-		kobject_put(main_kobj);
+		kobject_put(&instance->kobj);
 		goto err_out;
 	}
 
diff --git a/drivers/edac/edac_pci_sysfs.c b/drivers/edac/edac_pci_sysfs.c
index 446fef0a9399..971ec14742b9 100644
--- a/drivers/edac/edac_pci_sysfs.c
+++ b/drivers/edac/edac_pci_sysfs.c
@@ -176,7 +176,7 @@ static int edac_pci_create_instance_kobj(struct edac_pci_ctl_info *pci, int idx)
 				   edac_pci_top_main_kobj, "pci%d", idx);
 	if (err != 0) {
 		edac_dbg(2, "failed to register instance pci%d\n", idx);
-		kobject_put(edac_pci_top_main_kobj);
+		kobject_put(&pci->kobj);
 		goto error_out;
 	}
 
@@ -340,6 +340,7 @@ static int edac_pci_main_kobj_setup(void)
 	int err = -ENODEV;
 	const struct bus_type *edac_subsys;
 	struct device *dev_root;
+	bool kobj_initialized = false;
 
 	edac_dbg(0, "\n");
 
@@ -374,6 +375,7 @@ static int edac_pci_main_kobj_setup(void)
 		err = kobject_init_and_add(edac_pci_top_main_kobj,
 					   &ktype_edac_pci_main_kobj,
 					   &dev_root->kobj, "pci");
+		kobj_initialized = true;
 		put_device(dev_root);
 	}
 	if (err) {
@@ -392,7 +394,12 @@ static int edac_pci_main_kobj_setup(void)
 
 	/* Error unwind statck */
 kobject_init_and_add_fail:
-	kobject_put(edac_pci_top_main_kobj);
+	if (kobj_initialized) {
+		kobject_put(edac_pci_top_main_kobj);
+		goto decrement_count_fail;
+	}
+
+	kfree(edac_pci_top_main_kobj);
 
 kzalloc_fail:
 	module_put(THIS_MODULE);
-- 
2.43.0