[PATCH v3] regmap: debugfs: Fix a memory leak when calling regmap_attach_dev

[PATCH v3] regmap: debugfs: Fix a memory leak when calling regmap_attach_dev

[Xiaolei Wang]

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 <xiaolei.wang@windriver.com>
---
 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

Re: [PATCH v3] regmap: debugfs: Fix a memory leak when calling regmap_attach_dev

[Mark Brown]

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

RE: [PATCH v3] regmap: debugfs: Fix a memory leak when calling regmap_attach_dev

[Wang, Xiaolei]

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 <Markus.Elfring@web.de> 
Sent: Wednesday, December 30, 2020 5:49 PM
To: Wang, Xiaolei <Xiaolei.Wang@windriver.com>; kernel-janitors@vger.kernel.org
Cc: linux-kernel@vger.kernel.org; David Lechner <david@lechnology.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Mark Brown <broonie@kernel.org>; Rafael J. Wysocki <rafael@kernel.org>; Julia Lawall <julia.lawall@lip6.fr>
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

RE: [PATCH v3] regmap: debugfs: Fix a memory leak when calling regmap_attach_dev

[Wang, Xiaolei]

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 <Markus.Elfring@web.de> 
Sent: Wednesday, December 30, 2020 8:02 PM
To: Wang, Xiaolei <Xiaolei.Wang@windriver.com>; kernel-janitors@vger.kernel.org
Cc: linux-kernel@vger.kernel.org; David Lechner <david@lechnology.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Mark Brown <broonie@kernel.org>; Rafael J. Wysocki <rafael@kernel.org>; Julia Lawall <julia.lawall@lip6.fr>
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

Re: [PATCH 1/2] regmap: debugfs: Delete a misplaced pointer check in regmap_debugfs_init()

[Greg Kroah-Hartman]

On Mon, Jan 04, 2021 at 03:12:33PM +0100, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 4 Jan 2021 13:43:01 +0100

This does not belong in a changelog body of a patch :(

Re: [PATCH v3] regmap: debugfs: Fix a memory leak when calling regmap_attach_dev

[Greg Kroah-Hartman]

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

Re: [PATCH v3] regmap: debugfs: Fix a memory leak when calling regmap_attach_dev

[Mark Brown]

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

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.

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