From: Vladimir Zapolskiy <vz@mleia.com>
To: Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>
Cc: Wen Yang <wenyang@linux.alibaba.com>, linux-kernel@vger.kernel.org
Subject: [PATCH] regulator: fix memory leak on error path of regulator_register()
Date: Fri, 24 Jul 2020 03:50:13 +0300 [thread overview]
Message-ID: <20200724005013.23278-1-vz@mleia.com> (raw)
The change corrects registration and deregistration on error path
of a regulator, the problem was manifested by a reported memory
leak on deferred probe:
as3722-regulator as3722-regulator: regulator 13 register failed -517
# cat /sys/kernel/debug/kmemleak
unreferenced object 0xecc43740 (size 64):
comm "swapper/0", pid 1, jiffies 4294937640 (age 712.880s)
hex dump (first 32 bytes):
72 65 67 75 6c 61 74 6f 72 2e 32 34 00 5a 5a 5a regulator.24.ZZZ
5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZZZZZZZZZ
backtrace:
[<0c4c3d1c>] __kmalloc_track_caller+0x15c/0x2c0
[<40c0ad48>] kvasprintf+0x64/0xd4
[<109abd29>] kvasprintf_const+0x70/0x84
[<c4215946>] kobject_set_name_vargs+0x34/0xa8
[<62282ea2>] dev_set_name+0x40/0x64
[<a39b6757>] regulator_register+0x3a4/0x1344
[<16a9543f>] devm_regulator_register+0x4c/0x84
[<51a4c6a1>] as3722_regulator_probe+0x294/0x754
...
The memory leak problem was introduced as a side ef another fix in
regulator_register() error path, I believe that the proper fix is
to decouple device_register() function into its two compounds and
initialize a struct device before assigning any values to its fields
and then using it before actual registration of a device happens.
This lets to call put_device() safely after initialization, and, since
now a release callback is called, kfree(rdev->constraints) shall be
removed to exclude a double free condition.
Fixes: a3cde9534ebd ("regulator: core: fix regulator_register() error paths to properly release rdev")
Cc: Wen Yang <wenyang@linux.alibaba.com>
Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
drivers/regulator/core.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 03154f5b939f..720f28844795 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -5023,7 +5023,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
struct regulator_dev *rdev;
bool dangling_cfg_gpiod = false;
bool dangling_of_gpiod = false;
- bool reg_device_fail = false;
struct device *dev;
int ret, i;
@@ -5152,10 +5151,12 @@ regulator_register(const struct regulator_desc *regulator_desc,
}
/* register with sysfs */
+ device_initialize(&rdev->dev);
rdev->dev.class = ®ulator_class;
rdev->dev.parent = 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)
@@ -5206,12 +5207,9 @@ regulator_register(const struct regulator_desc *regulator_desc,
!rdev->desc->fixed_uV)
rdev->is_switch = true;
- dev_set_drvdata(&rdev->dev, rdev);
- ret = device_register(&rdev->dev);
- if (ret != 0) {
- reg_device_fail = true;
+ ret = device_add(&rdev->dev);
+ if (ret != 0)
goto unset_supplies;
- }
rdev_init_debugfs(rdev);
@@ -5233,17 +5231,15 @@ regulator_register(const struct regulator_desc *regulator_desc,
mutex_unlock(®ulator_list_mutex);
wash:
kfree(rdev->coupling_desc.coupled_rdevs);
- kfree(rdev->constraints);
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 (reg_device_fail)
- put_device(&rdev->dev);
- else
- kfree(rdev);
+ kfree(rdev);
kfree(config);
rinse:
if (dangling_cfg_gpiod)
--
2.24.0
next reply other threads:[~2020-07-24 0:50 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-24 0:50 Vladimir Zapolskiy [this message]
2020-07-24 14:37 ` Mark Brown
2020-07-24 7:09 Markus Elfring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200724005013.23278-1-vz@mleia.com \
--to=vz@mleia.com \
--cc=broonie@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=wenyang@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®