[PATCH 0/2] regulator/core: A quick fix for struct device handling

[PATCH 0/2] regulator/core: A quick fix for struct device handling

[Michał Mirosław]

When fixing a memory leak in commit d3c731564e09 ("regulator: plug of_node
leak in regulator_register()'s error path") it moved the device_initialize()
call earlier, but did not move the dev->class initialization.  The bug
was spotted and fixed by reverting part of the commit (in commit
5f4b204b6b81 "regulator: core: fix kobject release warning and memory
leak in regulator_register()") and introducing a different bug: now
early error paths use kfree(dev) instead of put_device() for an already
initialized struct device.

This series fixes the original bug and reverts the problematic fix.

Michał Mirosław (2):
  regulator/core: regulator_register: set device->class earlier
  Revert "regulator: core: fix kobject release warning and memory leak
    in regulator_register()"

 drivers/regulator/core.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

-- 
2.39.2

[PATCH 2/2] Revert "regulator: core: fix kobject release warning and memory leak in regulator_register()"

[Michał Mirosław]

This reverts commit 5f4b204b6b8153923d5be8002c5f7082985d153f.

Since rdev->dev now has a release() callback, the proper way of freeing
the initialized device can be restored.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/regulator/core.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 2f6ee5527cb0..0994caa5ad5c 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5724,15 +5724,11 @@ regulator_register(struct device *dev,
 	mutex_lock(&regulator_list_mutex);
 	regulator_ena_gpio_free(rdev);
 	mutex_unlock(&regulator_list_mutex);
-	put_device(&rdev->dev);
-	rdev = NULL;
 clean:
 	if (dangling_of_gpiod)
 		gpiod_put(config->ena_gpiod);
-	if (rdev && rdev->dev.of_node)
-		of_node_put(rdev->dev.of_node);
-	kfree(rdev);
 	kfree(config);
+	put_device(&rdev->dev);
 rinse:
 	if (dangling_cfg_gpiod)
 		gpiod_put(cfg->ena_gpiod);
-- 
2.39.2

Re: [PATCH 2/2] Revert "regulator: core: fix kobject release warning and memory leak in regulator_register()"

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

On Sun, Sep 17, 2023 at 11:35:14PM +0200, Michał Mirosław wrote:
> This reverts commit 5f4b204b6b8153923d5be8002c5f7082985d153f.
> 
> Since rdev->dev now has a release() callback, the proper way of freeing
> the initialized device can be restored.

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

Please include human readable descriptions of things like commits and
issues being discussed in e-mail in your mails, this makes them much
easier for humans to read especially when they have no internet access.
I do frequently catch up on my mail on flights or while otherwise
travelling so this is even more pressing for me than just being about
making things a bit easier to read.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 1/2] regulator/core: regulator_register: set device->class earlier

[Michał Mirosław]

device_initialize() calls for the `device` struct be freed with
`put_device()`. This requires a release() callback that is provided by
the regulator_class.

Fixes: d3c731564e09 ("regulator: plug of_node leak in regulator_register()'s error path")
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/regulator/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index d8e1caaf207e..2f6ee5527cb0 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5542,6 +5542,7 @@ regulator_register(struct device *dev,
 		goto rinse;
 	}
 	device_initialize(&rdev->dev);
+	rdev->dev.class = &regulator_class;
 	spin_lock_init(&rdev->err_lock);
 
 	/*
@@ -5603,7 +5604,6 @@ regulator_register(struct device *dev,
 		rdev->supply_name = regulator_desc->supply_name;
 
 	/* register with sysfs */
-	rdev->dev.class = &regulator_class;
 	rdev->dev.parent = config->dev;
 	dev_set_name(&rdev->dev, "regulator.%lu",
 		    (unsigned long) atomic_inc_return(&regulator_no));
-- 
2.39.2