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

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

[Xiaolei Wang]

This should be caused by
commit <9b947a13e7f6> ("<regmap: use debugfs even when no device>")

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

Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
---
 drivers/base/regmap/regmap-debugfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index 8dfac7f3ed7a..271142981f98 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -582,8 +582,10 @@ 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);
+		}
 		name = map->debugfs_name;
 	} else {
 		name = devname;
-- 
2.25.1

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

[Wang, Xiaolei]

Hi Markus

> This should be caused by
> commit <9b947a13e7f6> ("<regmap: use debugfs even when no device>")

> I suggest to omit angle brackets from such information.


> * Will another imperative wording become helpful for the commit description?

> * How do you think about to add the tag “Fixes”?

Do you mean so?

commit:
         9b947a13e7f6 ("regmap: use debugfs even when no device")

…
> +++ b/drivers/base/regmap/regmap-debugfs.c
…
> @@ -582,8 +582,10 @@ 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);
> +		}
…

> I propose to add another null pointer check together with corresponding exception handling.

Which one do you mean by "another null point"? Do you mean that if the call "regmap_debugfs_init" function is repeated, it returns directly?

Regards,
Xiaolei



-----Original Message-----
From: Markus Elfring <Markus.Elfring@web.de> 
Sent: Saturday, December 26, 2020 10:16 PM
To: Wang, Xiaolei <Xiaolei.Wang@windriver.com>; kernel-janitors@vger.kernel.org
Cc: linux-kernel@vger.kernel.org; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Mark Brown <broonie@kernel.org>; Rafael J. Wysocki <rafael@kernel.org>
Subject: Re: [PATCH] regmap: debugfs: fix a memory leak when calling regmap_attach_dev

> This should be caused by
> commit <9b947a13e7f6> ("<regmap: use debugfs even when no device>")

I suggest to omit angle brackets from such information.


* Will another imperative wording become helpful for the commit description?

* How do you think about to add the tag “Fixes”?


…
> +++ b/drivers/base/regmap/regmap-debugfs.c
…
> @@ -582,8 +582,10 @@ 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);
> +		}
…

I propose to add another null pointer check together with corresponding exception handling.

Regards,
Markus

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

[Wang, Xiaolei]

SGkgTWFya3VzDQoNCj4gVGhpcyBzaG91bGQgYmUgY2F1c2VkIGJ5DQo+IGNvbW1pdCA8OWI5NDdh
MTNlN2Y2PiAoIjxyZWdtYXA6IHVzZSBkZWJ1Z2ZzIGV2ZW4gd2hlbiBubyBkZXZpY2U+IikNCg0K
PiBJIHN1Z2dlc3QgdG8gb21pdCBhbmdsZSBicmFja2V0cyBmcm9tIHN1Y2ggaW5mb3JtYXRpb24u
DQoNCg0KPiAqIFdpbGwgYW5vdGhlciBpbXBlcmF0aXZlIHdvcmRpbmcgYmVjb21lIGhlbHBmdWwg
Zm9yIHRoZSBjb21taXQgZGVzY3JpcHRpb24/DQoNCj4gKiBIb3cgZG8geW91IHRoaW5rIGFib3V0
IHRvIGFkZCB0aGUgdGFnIOKAnEZpeGVz4oCdPw0KDQpEbyB5b3UgbWVhbiBzbz8NCg0KY29tbWl0
Og0KICAgICAgICAgOWI5NDdhMTNlN2Y2ICgicmVnbWFwOiB1c2UgZGVidWdmcyBldmVuIHdoZW4g
bm8gZGV2aWNlIikNCg0K4oCmDQo+ICsrKyBiL2RyaXZlcnMvYmFzZS9yZWdtYXAvcmVnbWFwLWRl
YnVnZnMuYw0K4oCmDQo+IEBAIC01ODIsOCArNTgyLDEwIEBAIHZvaWQgcmVnbWFwX2RlYnVnZnNf
aW5pdChzdHJ1Y3QgcmVnbWFwICptYXApDQo+ICAJCWRldm5hbWUgPSBkZXZfbmFtZShtYXAtPmRl
dik7DQo+DQo+ICAJaWYgKG5hbWUpIHsNCj4gLQkJbWFwLT5kZWJ1Z2ZzX25hbWUgPSBrYXNwcmlu
dGYoR0ZQX0tFUk5FTCwgIiVzLSVzIiwNCj4gKwkJaWYgKCFtYXAtPmRlYnVnZnNfbmFtZSkgew0K
PiArCQkJbWFwLT5kZWJ1Z2ZzX25hbWUgPSBrYXNwcmludGYoR0ZQX0tFUk5FTCwgIiVzLSVzIiwN
Cj4gIAkJCQkJICAgICAgZGV2bmFtZSwgbmFtZSk7DQo+ICsJCX0NCuKApg0KDQo+IEkgcHJvcG9z
ZSB0byBhZGQgYW5vdGhlciBudWxsIHBvaW50ZXIgY2hlY2sgdG9nZXRoZXIgd2l0aCBjb3JyZXNw
b25kaW5nIGV4Y2VwdGlvbiBoYW5kbGluZy4NCg0KV2hpY2ggb25lIGRvIHlvdSBtZWFuIGJ5ICJh
bm90aGVyIG51bGwgcG9pbnQiPyBEbyB5b3UgbWVhbiB0aGF0IGlmIHRoZSBjYWxsICJyZWdtYXBf
ZGVidWdmc19pbml0IiBmdW5jdGlvbiBpcyByZXBlYXRlZCwgaXQgcmV0dXJucyBkaXJlY3RseT8N
Cg0KUmVnYXJkcywNClhpYW9sZWkNCg0KDQoNCi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpG
cm9tOiBNYXJrdXMgRWxmcmluZyA8TWFya3VzLkVsZnJpbmdAd2ViLmRlPiANClNlbnQ6IFNhdHVy
ZGF5LCBEZWNlbWJlciAyNiwgMjAyMCAxMDoxNiBQTQ0KVG86IFdhbmcsIFhpYW9sZWkgPFhpYW9s
ZWkuV2FuZ0B3aW5kcml2ZXIuY29tPjsga2VybmVsLWphbml0b3JzQHZnZXIua2VybmVsLm9yZw0K
Q2M6IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7IEdyZWcgS3JvYWgtSGFydG1hbiA8Z3Jl
Z2toQGxpbnV4Zm91bmRhdGlvbi5vcmc+OyBNYXJrIEJyb3duIDxicm9vbmllQGtlcm5lbC5vcmc+
OyBSYWZhZWwgSi4gV3lzb2NraSA8cmFmYWVsQGtlcm5lbC5vcmc+DQpTdWJqZWN0OiBSZTogW1BB
VENIXSByZWdtYXA6IGRlYnVnZnM6IGZpeCBhIG1lbW9yeSBsZWFrIHdoZW4gY2FsbGluZyByZWdt
YXBfYXR0YWNoX2Rldg0KDQo+IFRoaXMgc2hvdWxkIGJlIGNhdXNlZCBieQ0KPiBjb21taXQgPDli
OTQ3YTEzZTdmNj4gKCI8cmVnbWFwOiB1c2UgZGVidWdmcyBldmVuIHdoZW4gbm8gZGV2aWNlPiIp
DQoNCkkgc3VnZ2VzdCB0byBvbWl0IGFuZ2xlIGJyYWNrZXRzIGZyb20gc3VjaCBpbmZvcm1hdGlv
bi4NCg0KDQoqIFdpbGwgYW5vdGhlciBpbXBlcmF0aXZlIHdvcmRpbmcgYmVjb21lIGhlbHBmdWwg
Zm9yIHRoZSBjb21taXQgZGVzY3JpcHRpb24/DQoNCiogSG93IGRvIHlvdSB0aGluayBhYm91dCB0
byBhZGQgdGhlIHRhZyDigJxGaXhlc+KAnT8NCg0KDQrigKYNCj4gKysrIGIvZHJpdmVycy9iYXNl
L3JlZ21hcC9yZWdtYXAtZGVidWdmcy5jDQrigKYNCj4gQEAgLTU4Miw4ICs1ODIsMTAgQEAgdm9p
ZCByZWdtYXBfZGVidWdmc19pbml0KHN0cnVjdCByZWdtYXAgKm1hcCkNCj4gIAkJZGV2bmFtZSA9
IGRldl9uYW1lKG1hcC0+ZGV2KTsNCj4NCj4gIAlpZiAobmFtZSkgew0KPiAtCQltYXAtPmRlYnVn
ZnNfbmFtZSA9IGthc3ByaW50ZihHRlBfS0VSTkVMLCAiJXMtJXMiLA0KPiArCQlpZiAoIW1hcC0+
ZGVidWdmc19uYW1lKSB7DQo+ICsJCQltYXAtPmRlYnVnZnNfbmFtZSA9IGthc3ByaW50ZihHRlBf
S0VSTkVMLCAiJXMtJXMiLA0KPiAgCQkJCQkgICAgICBkZXZuYW1lLCBuYW1lKTsNCj4gKwkJfQ0K
4oCmDQoNCkkgcHJvcG9zZSB0byBhZGQgYW5vdGhlciBudWxsIHBvaW50ZXIgY2hlY2sgdG9nZXRo
ZXIgd2l0aCBjb3JyZXNwb25kaW5nIGV4Y2VwdGlvbiBoYW5kbGluZy4NCg0KUmVnYXJkcywNCk1h
cmt1cw0K

RE: regmap: debugfs: fix a memory leak when calling regmap_attach_dev

[Wang, Xiaolei]

> Do you mean so?
>
> commit:
>          9b947a13e7f6 ("regmap: use debugfs even when no device")

> Please take another look at known information sources.

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=f838f8d2b694cf9d524dc4423e9dd2db13892f3f#n89
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=f838f8d2b694cf9d524dc4423e9dd2db13892f3f#n121



> Which one do you mean by "another null point"?

How do you think about to check if calls of the function “kasprintf” succeeded here?
https://elixir.bootlin.com/linux/v5.10.3/source/lib/kasprintf.c#L53

Will any more exception handling become relevant also for the implementation of the function “regmap_debugfs_init”?


-----Original Message-----
From: Markus Elfring <Markus.Elfring@web.de> 
Sent: Sunday, December 27, 2020 9:40 PM
To: Wang, Xiaolei <Xiaolei.Wang@windriver.com>; kernel-janitors@vger.kernel.org
Cc: linux-kernel@vger.kernel.org; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Mark Brown <broonie@kernel.org>; Rafael J. Wysocki <rafael@kernel.org>
Subject: Re: regmap: debugfs: fix a memory leak when calling regmap_attach_dev

> Do you mean so?
>
> commit:
>          9b947a13e7f6 ("regmap: use debugfs even when no device")

> Please take another look at known information sources.

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=f838f8d2b694cf9d524dc4423e9dd2db13892f3f#n89
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=f838f8d2b694cf9d524dc4423e9dd2db13892f3f#n121

> Which one do you mean by "another null point"?

> How do you think about to check if calls of the function “kasprintf” succeeded here?
> https://elixir.bootlin.com/linux/v5.10.3/source/lib/kasprintf.c#L53

> Will any more exception handling become relevant also for the implementation of the function “regmap_debugfs_init”?

Thanks for your suggestion, I will modify the commit log and add another null point check together with corresponding exception handling


Regards,
Xiaolei 

RE: regmap: debugfs: fix a memory leak when calling regmap_attach_dev

[Wang, Xiaolei]

PiBEbyB5b3UgbWVhbiBzbz8NCj4NCj4gY29tbWl0Og0KPiAgICAgICAgICA5Yjk0N2ExM2U3ZjYg
KCJyZWdtYXA6IHVzZSBkZWJ1Z2ZzIGV2ZW4gd2hlbiBubyBkZXZpY2UiKQ0KDQo+IFBsZWFzZSB0
YWtlIGFub3RoZXIgbG9vayBhdCBrbm93biBpbmZvcm1hdGlvbiBzb3VyY2VzLg0KDQo+IGh0dHBz
Oi8vZ2l0Lmtlcm5lbC5vcmcvcHViL3NjbS9saW51eC9rZXJuZWwvZ2l0L3RvcnZhbGRzL2xpbnV4
LmdpdC90cmVlL0RvY3VtZW50YXRpb24vcHJvY2Vzcy9zdWJtaXR0aW5nLXBhdGNoZXMucnN0P2lk
PWY4MzhmOGQyYjY5NGNmOWQ1MjRkYzQ0MjNlOWRkMmRiMTM4OTJmM2Yjbjg5DQo+IGh0dHBzOi8v
Z2l0Lmtlcm5lbC5vcmcvcHViL3NjbS9saW51eC9rZXJuZWwvZ2l0L3RvcnZhbGRzL2xpbnV4Lmdp
dC90cmVlL0RvY3VtZW50YXRpb24vcHJvY2Vzcy9zdWJtaXR0aW5nLXBhdGNoZXMucnN0P2lkPWY4
MzhmOGQyYjY5NGNmOWQ1MjRkYzQ0MjNlOWRkMmRiMTM4OTJmM2YjbjEyMQ0KDQoNCg0KPiBXaGlj
aCBvbmUgZG8geW91IG1lYW4gYnkgImFub3RoZXIgbnVsbCBwb2ludCI/DQoNCkhvdyBkbyB5b3Ug
dGhpbmsgYWJvdXQgdG8gY2hlY2sgaWYgY2FsbHMgb2YgdGhlIGZ1bmN0aW9uIOKAnGthc3ByaW50
ZuKAnSBzdWNjZWVkZWQgaGVyZT8NCmh0dHBzOi8vZWxpeGlyLmJvb3RsaW4uY29tL2xpbnV4L3Y1
LjEwLjMvc291cmNlL2xpYi9rYXNwcmludGYuYyNMNTMNCg0KV2lsbCBhbnkgbW9yZSBleGNlcHRp
b24gaGFuZGxpbmcgYmVjb21lIHJlbGV2YW50IGFsc28gZm9yIHRoZSBpbXBsZW1lbnRhdGlvbiBv
ZiB0aGUgZnVuY3Rpb24g4oCccmVnbWFwX2RlYnVnZnNfaW5pdOKAnT8NCg0KDQotLS0tLU9yaWdp
bmFsIE1lc3NhZ2UtLS0tLQ0KRnJvbTogTWFya3VzIEVsZnJpbmcgPE1hcmt1cy5FbGZyaW5nQHdl
Yi5kZT4gDQpTZW50OiBTdW5kYXksIERlY2VtYmVyIDI3LCAyMDIwIDk6NDAgUE0NClRvOiBXYW5n
LCBYaWFvbGVpIDxYaWFvbGVpLldhbmdAd2luZHJpdmVyLmNvbT47IGtlcm5lbC1qYW5pdG9yc0B2
Z2VyLmtlcm5lbC5vcmcNCkNjOiBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnOyBHcmVnIEty
b2FoLUhhcnRtYW4gPGdyZWdraEBsaW51eGZvdW5kYXRpb24ub3JnPjsgTWFyayBCcm93biA8YnJv
b25pZUBrZXJuZWwub3JnPjsgUmFmYWVsIEouIFd5c29ja2kgPHJhZmFlbEBrZXJuZWwub3JnPg0K
U3ViamVjdDogUmU6IHJlZ21hcDogZGVidWdmczogZml4IGEgbWVtb3J5IGxlYWsgd2hlbiBjYWxs
aW5nIHJlZ21hcF9hdHRhY2hfZGV2DQoNCj4gRG8geW91IG1lYW4gc28/DQo+DQo+IGNvbW1pdDoN
Cj4gICAgICAgICAgOWI5NDdhMTNlN2Y2ICgicmVnbWFwOiB1c2UgZGVidWdmcyBldmVuIHdoZW4g
bm8gZGV2aWNlIikNCg0KPiBQbGVhc2UgdGFrZSBhbm90aGVyIGxvb2sgYXQga25vd24gaW5mb3Jt
YXRpb24gc291cmNlcy4NCg0KPiBodHRwczovL2dpdC5rZXJuZWwub3JnL3B1Yi9zY20vbGludXgv
a2VybmVsL2dpdC90b3J2YWxkcy9saW51eC5naXQvdHJlZS9Eb2N1bWVudGF0aW9uL3Byb2Nlc3Mv
c3VibWl0dGluZy1wYXRjaGVzLnJzdD9pZD1mODM4ZjhkMmI2OTRjZjlkNTI0ZGM0NDIzZTlkZDJk
YjEzODkyZjNmI244OQ0KPiBodHRwczovL2dpdC5rZXJuZWwub3JnL3B1Yi9zY20vbGludXgva2Vy
bmVsL2dpdC90b3J2YWxkcy9saW51eC5naXQvdHJlZS9Eb2N1bWVudGF0aW9uL3Byb2Nlc3Mvc3Vi
bWl0dGluZy1wYXRjaGVzLnJzdD9pZD1mODM4ZjhkMmI2OTRjZjlkNTI0ZGM0NDIzZTlkZDJkYjEz
ODkyZjNmI24xMjENCg0KPiBXaGljaCBvbmUgZG8geW91IG1lYW4gYnkgImFub3RoZXIgbnVsbCBw
b2ludCI/DQoNCj4gSG93IGRvIHlvdSB0aGluayBhYm91dCB0byBjaGVjayBpZiBjYWxscyBvZiB0
aGUgZnVuY3Rpb24g4oCca2FzcHJpbnRm4oCdIHN1Y2NlZWRlZCBoZXJlPw0KPiBodHRwczovL2Vs
aXhpci5ib290bGluLmNvbS9saW51eC92NS4xMC4zL3NvdXJjZS9saWIva2FzcHJpbnRmLmMjTDUz
DQoNCj4gV2lsbCBhbnkgbW9yZSBleGNlcHRpb24gaGFuZGxpbmcgYmVjb21lIHJlbGV2YW50IGFs
c28gZm9yIHRoZSBpbXBsZW1lbnRhdGlvbiBvZiB0aGUgZnVuY3Rpb24g4oCccmVnbWFwX2RlYnVn
ZnNfaW5pdOKAnT8NCg0KVGhhbmtzIGZvciB5b3VyIHN1Z2dlc3Rpb24sIEkgd2lsbCBtb2RpZnkg
dGhlIGNvbW1pdCBsb2cgYW5kIGFkZCBhbm90aGVyIG51bGwgcG9pbnQgY2hlY2sgdG9nZXRoZXIg
d2l0aCBjb3JyZXNwb25kaW5nIGV4Y2VwdGlvbiBoYW5kbGluZw0KDQoNClJlZ2FyZHMsDQpYaWFv
bGVpIA0K