From cffa4b2122f5f3e53cf3d529bbc74651f95856d5 Mon Sep 17 00:00:00 2001 From: Xiaolei Wang Date: Tue, 29 Dec 2020 18:50:46 +0800 Subject: regmap: debugfs: Fix a memory leak when calling regmap_attach_dev 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: [] kasprintf+0x2c/0x54 [<6ad3bbc2>] regmap_debugfs_init+0xdc/0x2fc [] __regmap_init+0xc38/0xd88 [<1f7e0609>] of_syscon_register+0x168/0x294 [<735e8766>] device_node_get_regmap+0x6c/0x98 [] imx6ul_init_machine+0x20/0x88 [<0456565b>] customize_machine+0x1c/0x30 [] 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 Link: https://lore.kernel.org/r/20201229105046.41984-1-xiaolei.wang@windriver.com Signed-off-by: Mark Brown --- drivers/base/regmap/regmap-debugfs.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'drivers/base') 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++; } -- cgit v1.2.3 From f6bcb4c7f366905b66ce8ffca7190118244bb642 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 5 Jan 2021 14:42:29 +0300 Subject: regmap: debugfs: Fix a reversed if statement in regmap_debugfs_init() This code will leak "map->debugfs_name" because the if statement is reversed so it only frees NULL pointers instead of non-NULL. In fact the if statement is not required and should just be removed because kfree() accepts NULL pointers. Fixes: cffa4b2122f5 ("regmap: debugfs: Fix a memory leak when calling regmap_attach_dev") Signed-off-by: Dan Carpenter Link: https://lore.kernel.org/r/X/RQpfAwRdLg0GqQ@mwanda Signed-off-by: Mark Brown --- drivers/base/regmap/regmap-debugfs.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c index bf03cd343be2..ff2ee87987c7 100644 --- a/drivers/base/regmap/regmap-debugfs.c +++ b/drivers/base/regmap/regmap-debugfs.c @@ -594,9 +594,7 @@ void regmap_debugfs_init(struct regmap *map) } if (!strcmp(name, "dummy")) { - if (!map->debugfs_name) - kfree(map->debugfs_name); - + kfree(map->debugfs_name); map->debugfs_name = kasprintf(GFP_KERNEL, "dummy%d", dummy_index); if (!map->debugfs_name) -- cgit v1.2.3 From 47f4469970d8861bc06d2d4d45ac8200ff07c693 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Tue, 5 Jan 2021 17:11:45 +0800 Subject: Revert "device property: Keep secondary firmware node secondary by type" While commit d5dcce0c414f ("device property: Keep secondary firmware node secondary by type") describes everything correct in its commit message, the change it made does the opposite and original commit c15e1bdda436 ("device property: Fix the secondary firmware node handling in set_primary_fwnode()") was fully correct. Revert the former one here and improve documentation in the next patch. Fixes: d5dcce0c414f ("device property: Keep secondary firmware node secondary by type") Signed-off-by: Bard Liao Reviewed-by: Andy Shevchenko Reviewed-by: Heikki Krogerus Cc: 5.10+ # 5.10+ Signed-off-by: Rafael J. Wysocki --- drivers/base/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index 25e08e5f40bd..51b9545a050b 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -4433,7 +4433,7 @@ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode) if (fwnode_is_primary(fn)) { dev->fwnode = fn->secondary; if (!(parent && fn == parent->fwnode)) - fn->secondary = ERR_PTR(-ENODEV); + fn->secondary = NULL; } else { dev->fwnode = NULL; } -- cgit v1.2.3 From 3f7bddaf5d5a83aa2eb1e6d72db221d3ec43c813 Mon Sep 17 00:00:00 2001 From: Bard Liao Date: Tue, 5 Jan 2021 17:11:46 +0800 Subject: device property: add description of fwnode cases There are only four valid fwnode cases which are - primary --> secondary --> -ENODEV - primary --> NULL - secondary --> -ENODEV - NULL dev->fwnode should be converted between the 4 cases above no matter how/when set_primary_fwnode() and set_secondary_fwnode() are called. Describe it in the code so people will keep it in mind. Signed-off-by: Bard Liao Reviewed-by: Andy Shevchenko Reviewed-by: Heikki Krogerus [ rjw: Comment edit ] Signed-off-by: Rafael J. Wysocki --- drivers/base/core.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/base') diff --git a/drivers/base/core.c b/drivers/base/core.c index 51b9545a050b..14f165816742 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -4414,6 +4414,12 @@ static inline bool fwnode_is_primary(struct fwnode_handle *fwnode) * * Set the device's firmware node pointer to @fwnode, but if a secondary * firmware node of the device is present, preserve it. + * + * Valid fwnode cases are: + * - primary --> secondary --> -ENODEV + * - primary --> NULL + * - secondary --> -ENODEV + * - NULL */ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode) { @@ -4432,6 +4438,7 @@ void set_primary_fwnode(struct device *dev, struct fwnode_handle *fwnode) } else { if (fwnode_is_primary(fn)) { dev->fwnode = fn->secondary; + /* Set fn->secondary = NULL, so fn remains the primary fwnode */ if (!(parent && fn == parent->fwnode)) fn->secondary = NULL; } else { -- cgit v1.2.3