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()") but
introducing a different bug: now early error paths use `kfree(dev)`
instead of `put_device()` for an already initialized `struct device`.
Move the missing assignments to just after `device_initialize()`.
Fixes: d3c731564e09 ("regulator: plug of_node leak in regulator_register()'s error path")
Signed-off-by: Michał Mirosław <[email protected]>
---
v2: move dev_set_drvdata() as it is used by regulator_dev_release()
+ move cover letter explanation to this patch
---
drivers/regulator/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index d8e1caaf207e..2820badc7a12 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5542,6 +5542,8 @@ regulator_register(struct device *dev,
goto rinse;
}
device_initialize(&rdev->dev);
+ dev_set_drvdata(&rdev->dev, rdev);
+ rdev->dev.class = ®ulator_class;
spin_lock_init(&rdev->err_lock);
/*
@@ -5603,11 +5605,9 @@ regulator_register(struct device *dev,
rdev->supply_name = regulator_desc->supply_name;
/* register with sysfs */
- rdev->dev.class = ®ulator_class;
rdev->dev.parent = config->dev;
dev_set_name(&rdev->dev, "regulator.%lu",
(unsigned long) atomic_inc_return(®ulator_no));
- dev_set_drvdata(&rdev->dev, rdev);
/* set regulator constraints */
if (init_data)
--
2.39.2
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 <[email protected]>
---
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 2820badc7a12..3137e40fcd3e 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5724,15 +5724,11 @@ regulator_register(struct device *dev,
mutex_lock(®ulator_list_mutex);
regulator_ena_gpio_free(rdev);
mutex_unlock(®ulator_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
On Tue, Sep 19, 2023 at 12:50:27AM +0200, Michał Mirosław wrote:
> This reverts commit 5f4b204b6b8153923d5be8002c5f7082985d153f.
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.
On Tue, 19 Sep 2023 00:50:26 +0200, Michał Mirosław wrote:
> 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()") but
> introducing a different bug: now early error paths use `kfree(dev)`
> instead of `put_device()` for an already initialized `struct device`.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
Thanks!
[1/2] regulator/core: regulator_register: set device->class earlier
commit: 8adb4e647a83cb5928c05dae95b010224aea0705
[2/2] regulator/core: Revert "fix kobject release warning and memory leak in regulator_register()"
commit: 6e800968f6a715c0661716d2ec5e1f56ed9f9c08
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark