After initializing the regmap through
syscon_regmap_lookup_by_compatible, then regmap_attach_dev to the
device, because the debugfs_name has been allocated, there is no
need to redistribute it again
unreferenced object 0xd8399b80 (size 64):
comm "swapper/0", pid 1, jiffies 4294937641 (age 278.590s)
hex dump (first 32 bytes):
64 75 6d 6d 79 2d 69 6f 6d 75 78 63 2d 67 70 72
dummy-iomuxc-gpr
40 32 30 65 34 30 30 30 00 7f 52 5b d8 7e 42 69
@20e4000..R[.~Bi
backtrace:
[<ca384d6f>] kasprintf+0x2c/0x54
[<6ad3bbc2>] regmap_debugfs_init+0xdc/0x2fc
[<bc4181da>] __regmap_init+0xc38/0xd88
[<1f7e0609>] of_syscon_register+0x168/0x294
[<735e8766>] device_node_get_regmap+0x6c/0x98
[<d96c8982>] imx6ul_init_machine+0x20/0x88
[<0456565b>] customize_machine+0x1c/0x30
[<d07393d8>] do_one_initcall+0x80/0x3ac
[<7e584867>] kernel_init_freeable+0x170/0x1f0
[<80074741>] kernel_init+0x8/0x120
[<285d6f28>] ret_from_fork+0x14/0x20
[<00000000>] 0x0
Fixes: 9b947a13e7f6 ("regmap: use debugfs even when no device")
Signed-off-by: Xiaolei Wang <[email protected]>
---
drivers/base/regmap/regmap-debugfs.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index 8dfac7f3ed7a..bf03cd343be2 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -582,18 +582,25 @@ void regmap_debugfs_init(struct regmap *map)
devname = dev_name(map->dev);
if (name) {
- map->debugfs_name = kasprintf(GFP_KERNEL, "%s-%s",
+ if (!map->debugfs_name) {
+ map->debugfs_name = kasprintf(GFP_KERNEL, "%s-%s",
devname, name);
+ if (!map->debugfs_name)
+ return;
+ }
name = map->debugfs_name;
} else {
name = devname;
}
if (!strcmp(name, "dummy")) {
- kfree(map->debugfs_name);
+ if (!map->debugfs_name)
+ kfree(map->debugfs_name);
map->debugfs_name = kasprintf(GFP_KERNEL, "dummy%d",
dummy_index);
+ if (!map->debugfs_name)
+ return;
name = map->debugfs_name;
dummy_index++;
}
--
2.25.1
On Tue, 29 Dec 2020 18:50:46 +0800, Xiaolei Wang wrote:
> After initializing the regmap through
> syscon_regmap_lookup_by_compatible, then regmap_attach_dev to the
> device, because the debugfs_name has been allocated, there is no
> need to redistribute it again
>
> unreferenced object 0xd8399b80 (size 64):
> comm "swapper/0", pid 1, jiffies 4294937641 (age 278.590s)
> hex dump (first 32 bytes):
> 64 75 6d 6d 79 2d 69 6f 6d 75 78 63 2d 67 70 72
> dummy-iomuxc-gpr
> 40 32 30 65 34 30 30 30 00 7f 52 5b d8 7e 42 69
> @20e4000..R[.~Bi
> backtrace:
> [<ca384d6f>] kasprintf+0x2c/0x54
> [<6ad3bbc2>] regmap_debugfs_init+0xdc/0x2fc
> [<bc4181da>] __regmap_init+0xc38/0xd88
> [<1f7e0609>] of_syscon_register+0x168/0x294
> [<735e8766>] device_node_get_regmap+0x6c/0x98
> [<d96c8982>] imx6ul_init_machine+0x20/0x88
> [<0456565b>] customize_machine+0x1c/0x30
> [<d07393d8>] do_one_initcall+0x80/0x3ac
> [<7e584867>] kernel_init_freeable+0x170/0x1f0
> [<80074741>] kernel_init+0x8/0x120
> [<285d6f28>] ret_from_fork+0x14/0x20
> [<00000000>] 0x0
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next
Thanks!
[1/1] regmap: debugfs: Fix a memory leak when calling regmap_attach_dev
commit: cffa4b2122f5f3e53cf3d529bbc74651f95856d5
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
Hi Markus
Thank you very much, very good suggestion, do I need to re-send a patch to fix this problem, or modify the previous patch and send it again?
Thanks
Xiaolei
…
> +++ b/drivers/base/regmap/regmap-debugfs.c
> @@ -582,18 +582,25 @@ void regmap_debugfs_init(struct regmap *map)
…
> + map->debugfs_name = kasprintf(GFP_KERNEL, "%s-%s",
> devname, name);
> I suggest to reconsider the alignment for function call parameters for this patch.
…
> + if (!map->debugfs_name)
> + kfree(map->debugfs_name);
> Such a null pointer check is redundant.
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/free/ifnullfree.cocci?id=139711f033f636cc78b6aaf7363252241b9698ef#n2
-----Original Message-----
From: Markus Elfring <[email protected]>
Sent: Wednesday, December 30, 2020 5:49 PM
To: Wang, Xiaolei <[email protected]>; [email protected]
Cc: [email protected]; David Lechner <[email protected]>; Greg Kroah-Hartman <[email protected]>; Mark Brown <[email protected]>; Rafael J. Wysocki <[email protected]>; Julia Lawall <[email protected]>
Subject: Re: [PATCH v3] regmap: debugfs: Fix a memory leak when calling regmap_attach_dev
…
> +++ b/drivers/base/regmap/regmap-debugfs.c
> @@ -582,18 +582,25 @@ void regmap_debugfs_init(struct regmap *map)
…
> + map->debugfs_name = kasprintf(GFP_KERNEL, "%s-%s",
> devname, name);
I suggest to reconsider the alignment for function call parameters for this patch.
…
> + if (!map->debugfs_name)
> + kfree(map->debugfs_name);
Such a null pointer check is redundant.
See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/free/ifnullfree.cocci?id=139711f033f636cc78b6aaf7363252241b9698ef#n2
Regards,
Markus
Hi Markus
> Thank you very much, very good suggestion,
> Thanks for another positive feedback.
> do I need to re-send a patch to fix this problem, or modify the previous patch and send it again?
> Please convince the involved contributors to integrate a corrected patch version.
Do you mean that I should correct the original patch and explain my changes and send it out?
> * Better indentation.
* …
>> + if (!map->debugfs_name)
>> + kfree(map->debugfs_name);
> Would this questionable null pointer check result in a memory leak?
if (!map->debugfs_name)
kfree(map->debugfs_name);
This null pointer check is not in the memory leak
Thanks
xiaolei
-----Original Message-----
From: Markus Elfring <[email protected]>
Sent: Wednesday, December 30, 2020 8:02 PM
To: Wang, Xiaolei <[email protected]>; [email protected]
Cc: [email protected]; David Lechner <[email protected]>; Greg Kroah-Hartman <[email protected]>; Mark Brown <[email protected]>; Rafael J. Wysocki <[email protected]>; Julia Lawall <[email protected]>
Subject: Re: [PATCH v3] regmap: debugfs: Fix a memory leak when calling regmap_attach_dev
> Thank you very much, very good suggestion,
Thanks for another positive feedback.
> do I need to re-send a patch to fix this problem, or modify the previous patch and send it again?
Please convince the involved contributors to integrate a corrected patch version.
* Better indentation.
* …
>> + if (!map->debugfs_name)
>> + kfree(map->debugfs_name);
Would this questionable null pointer check result in a memory leak?
Regards,
Markus
On Wed, Dec 30, 2020 at 01:01:42PM +0100, Markus Elfring wrote:
> > Thank you very much, very good suggestion,
>
> Thanks for another positive feedback.
>
>
> > do I need to re-send a patch to fix this problem, or modify the previous patch and send it again?
>
> Please convince the involved contributors to integrate a corrected patch version.
>
> * Better indentation.
>
> * …
> >> + if (!map->debugfs_name)
> >> + kfree(map->debugfs_name);
>
> Would this questionable null pointer check result in a memory leak?
Hi,
This is the semi-friendly patch-bot of Greg Kroah-Hartman.
Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list. I strongly suggest that you not do this anymore. Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.
Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all. The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback. Please feel free to also ignore emails
from them.
thanks,
greg k-h's patch email bot
On Wed, Dec 30, 2020 at 10:14:22AM +0000, Wang, Xiaolei wrote:
> Hi Markus
>
> Thank you very much, very good suggestion, do I need to re-send a patch to fix this problem, or modify the previous patch and send it again?
Please feel free to ignore Markus.