Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Juergen Borleis <jbe@pengutronix.de>
To:     linux-kernel@vger.kernel.org
Cc:     Mark Brown <broonie@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J . Wysocki" <rafael@kernel.org>, kernel@pengutronix.de
Subject: [PATCH] regmap: debugfs: really fix memory leak in regmap_debugfs_init()
Date:   Tue, 30 Jun 2020 12:06:50 +0200
Message-Id: <20200630100650.23052-1-jbe@pengutronix.de>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

A different approach tried to fix the memory leak in the
regmap_debugfs_init() function about one year ago. But it added the missing
kfree() at the wrong code path. Even with this approach applied,
'kmemleak' still finds this memory leak (at least on my i.MX6Q SoC):

  unreferenced object 0xec548c80 (size 64):
  comm "swapper/0", pid 1, jiffies 4294937356 (age 3385.690s)
  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 30 30 30 30 00 7d cc fa 7c df bf 7d  @20e0000.}..|..}
  backtrace:
    [<703904e1>] __kmalloc_track_caller+0x1dc/0x43c
    [<523053d0>] kvasprintf+0x64/0xd8
    [<0d94bbe5>] kasprintf+0x38/0x5c
    [<4ba65bc1>] regmap_debugfs_init+0xa8/0x330
    [<dfcf84e9>] __regmap_init+0x890/0xd84
    [<750d0959>] __regmap_init_mmio_clk+0x50/0x5c
    [<02a69967>] of_syscon_register+0x194/0x2a8
    [<8d9f337a>] device_node_get_regmap+0x9c/0xa4
    [<97914327>] syscon_node_to_regmap+0x40/0x44
    [<b1677f4e>] syscon_regmap_lookup_by_compatible+0x30/0x3c
    [<0b2d4178>] imx6_pm_common_init.constprop.0+0x4c/0x7c
    [<76253dc8>] imx6q_pm_init+0x18/0x1c
    [<1d2cec28>] imx6q_init_machine+0x120/0x2bc
    [<7386bff5>] customize_machine+0x30/0x38
    [<bc13c21d>] do_one_initcall+0x68/0x2f8
    [<a31f6aea>] kernel_init_freeable+0x24c/0x2f0

For this specific i.MX device the regmap_debugfs_init() function is called
twice for the same object:

- first call: with name = "iomuxc-gpr@20e0000", but the underlying device
  has no name yet. So, regmap_debugfs_init() allocates the string
  "dummy-iomuxc-gpr@20e0000" into 'map->debugfs_name' (and this string is
  the one 'kmemleak' reports in its output).

- second call: with name = "gpr" and the underlying device now has the
  name "20e0000.iomuxc". So, regmap_debugfs_init() again allocates
  a string "20e0000.iomuxc-gpr" into 'map->debugfs_name'.

Since the underlying regmap structure is allocated via kzalloc() the
'map->debugfs_name' member should be NULL on the first call. So, it should
be safe to kfree() this member unconditionally prior allocating a new
string for this member.

Fixes: 2899872b627e ("regmap: debugfs: Fix memory leak in regmap_debugfs_init")

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---
 drivers/base/regmap/regmap-debugfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index 089e5dc7144a..81c8fff8c09f 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -577,6 +577,7 @@ void regmap_debugfs_init(struct regmap *map, const char *name)
 		devname = dev_name(map->dev);
 
 	if (name) {
+		kfree(map->debugfs_name);
 		map->debugfs_name = kasprintf(GFP_KERNEL, "%s-%s",
 					      devname, name);
 		name = map->debugfs_name;
-- 
2.20.1