mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/4] fix some memory leaks
@ 2025-11-07 12:00 Sascha Hauer
  2025-11-07 12:00 ` [PATCH 1/4] tlv: fix memory leak Sascha Hauer
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Sascha Hauer @ 2025-11-07 12:00 UTC (permalink / raw)
  To: BAREBOX

This series fixes some memory leaks I stumbled upon

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (4):
      tlv: fix memory leak
      of: free of_fixup in of_unregister_fixup()
      bobject: free object name in bobject_del()
      bfetch: fix memory leak

 commands/bfetch.c | 28 +++++++++++++++++++++++-----
 common/oftree.c   |  1 +
 common/tlv/bus.c  |  6 +++++-
 include/bobject.h |  4 ----
 lib/bobject.c     |  7 +++----
 5 files changed, 32 insertions(+), 14 deletions(-)
---
base-commit: 5789cb9767ba53b757bcb6bb12a676ff9ea69d34
change-id: 20251107-memleaks-365903cca3c3

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/4] tlv: fix memory leak
  2025-11-07 12:00 [PATCH 0/4] fix some memory leaks Sascha Hauer
@ 2025-11-07 12:00 ` Sascha Hauer
  2025-11-07 12:00 ` [PATCH 2/4] of: free of_fixup in of_unregister_fixup() Sascha Hauer
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2025-11-07 12:00 UTC (permalink / raw)
  To: BAREBOX

of_new_node(of_new_node(NULL, NULL), ...) leaks memory of the parent
node which is never freed. Fix this by allocating a reusable parent
node in tlv_bus_register().

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/tlv/bus.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/common/tlv/bus.c b/common/tlv/bus.c
index 4dbffd365667b258991e5ab01a99be4ee225fee2..a59f8d297a27975a43dd98230e04593150bf250d 100644
--- a/common/tlv/bus.c
+++ b/common/tlv/bus.c
@@ -14,6 +14,8 @@ static void tlv_devinfo(struct device *dev)
 	printf("Magic: %08x\n", tlvdev->magic);
 }
 
+static struct device_node *tlv_parent_node;
+
 struct tlv_device *tlv_register_device(struct tlv_header *header,
 				       struct device *parent)
 {
@@ -42,7 +44,7 @@ struct tlv_device *tlv_register_device(struct tlv_header *header,
 	if (!name)
 		dev_set_name(dev, "tlv%u", id++);
 
-	dev->device_node = of_new_node(of_new_node(NULL, NULL), dev_name(dev));
+	dev->device_node = of_new_node(tlv_parent_node, dev_name(dev));
 	dev->device_node->dev = dev;
 	register_device(dev);
 
@@ -123,6 +125,8 @@ static int tlv_bus_register(void)
 {
 	int ret;
 
+	tlv_parent_node = of_new_node(NULL, NULL);
+
 	ret = bus_register(&tlv_bus);
 	if (ret)
 		return ret;

-- 
2.47.3




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/4] of: free of_fixup in of_unregister_fixup()
  2025-11-07 12:00 [PATCH 0/4] fix some memory leaks Sascha Hauer
  2025-11-07 12:00 ` [PATCH 1/4] tlv: fix memory leak Sascha Hauer
@ 2025-11-07 12:00 ` Sascha Hauer
  2025-11-07 12:00 ` [PATCH 3/4] bobject: free object name in bobject_del() Sascha Hauer
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2025-11-07 12:00 UTC (permalink / raw)
  To: BAREBOX

The fixup is allocated in of_register_fixup(), so must be freed in
of_unregister_fixup().

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/oftree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/oftree.c b/common/oftree.c
index c22793de89fb43f3e9a6c2b0905cbf271fd7cb2b..2bf1a63e491540100839e234f891aef597fae0e0 100644
--- a/common/oftree.c
+++ b/common/oftree.c
@@ -383,6 +383,7 @@ int of_unregister_fixup(int (*fixup)(struct device_node *, void *),
 	list_for_each_entry(of_fixup, &of_fixup_list, list) {
 		if (of_fixup->fixup == fixup && of_fixup->context == context) {
 			list_del(&of_fixup->list);
+			free(of_fixup);
 			return 0;
 		}
 	}

-- 
2.47.3




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/4] bobject: free object name in bobject_del()
  2025-11-07 12:00 [PATCH 0/4] fix some memory leaks Sascha Hauer
  2025-11-07 12:00 ` [PATCH 1/4] tlv: fix memory leak Sascha Hauer
  2025-11-07 12:00 ` [PATCH 2/4] of: free of_fixup in of_unregister_fixup() Sascha Hauer
@ 2025-11-07 12:00 ` Sascha Hauer
  2025-11-10 13:48   ` Sascha Hauer
  2025-11-07 12:00 ` [PATCH 4/4] bfetch: fix memory leak Sascha Hauer
  2025-11-10 13:46 ` [PATCH 0/4] fix some memory leaks Sascha Hauer
  4 siblings, 1 reply; 7+ messages in thread
From: Sascha Hauer @ 2025-11-07 12:00 UTC (permalink / raw)
  To: BAREBOX

We have bobject_free() which removes all parameters and frees the
bobject. This works for bobjects which have been allocated with
bobject_alloc(). struct device has a bobject embedded and is freed
along with the device, so bobject_free() is inappropriate and
bobject_del() is called instead. This however doesn't free the bobjects
name which then leaks when the device is freed.

To fix this move the freeing of the bobject name to bobject_del(). To
make this work with CONFIG_PARAMETER disabled drop the static inline
wrapper.

While at it fix the function name in the bobject_del() function
description.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/bobject.h | 4 ----
 lib/bobject.c     | 7 +++----
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/include/bobject.h b/include/bobject.h
index 34cf9d62bc6e3373b7b6b6073640aeceadda7c8f..76c9158b71a6a68994ee3491c781a6d7d7f2f9ac 100644
--- a/include/bobject.h
+++ b/include/bobject.h
@@ -39,10 +39,6 @@ static inline void bobject_init(struct bobject *bobj)
 struct bobject *bobject_alloc(const char *name);
 void bobject_free(struct bobject *bobj);
 
-#ifdef CONFIG_PARAMETER
 void bobject_del(struct bobject *bobj);
-#else
-static inline void bobject_del(struct bobject *bobj) { }
-#endif
 
 #endif
diff --git a/lib/bobject.c b/lib/bobject.c
index eb140b90a2d58db2497ba4b95e1995f4965a1d34..373575eade0c1ceb4c2198cfab1005a8e5230caa 100644
--- a/lib/bobject.c
+++ b/lib/bobject.c
@@ -51,15 +51,13 @@ void bobject_free(struct bobject *bobj)
 	if (!bobj)
 		return;
 
-	free(bobj->name);
 	bobject_del(bobj);
 	free(bobj);
 }
 EXPORT_SYMBOL_GPL(bobject_free);
 
-#ifdef CONFIG_PARAMETER
 /**
- * bobject_remove_parameters - remove all parameters from a bobject and free their
+ * bobject_del - remove all parameters from a bobject and free their
  * memory
  * @param bobject	The barebox object
  */
@@ -69,6 +67,7 @@ void bobject_del(struct bobject *bobj)
 
 	list_for_each_entry_safe(p, n, &bobj->parameters, list)
 		param_remove(p);
+
+	free(bobj->name);
 }
 EXPORT_SYMBOL(bobject_del);
-#endif

-- 
2.47.3




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 4/4] bfetch: fix memory leak
  2025-11-07 12:00 [PATCH 0/4] fix some memory leaks Sascha Hauer
                   ` (2 preceding siblings ...)
  2025-11-07 12:00 ` [PATCH 3/4] bobject: free object name in bobject_del() Sascha Hauer
@ 2025-11-07 12:00 ` Sascha Hauer
  2025-11-10 13:46 ` [PATCH 0/4] fix some memory leaks Sascha Hauer
  4 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2025-11-07 12:00 UTC (permalink / raw)
  To: BAREBOX

string_list_join() returns an allocated string. Free it after usage.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/bfetch.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/commands/bfetch.c b/commands/bfetch.c
index a44d5a000667c0802a3795bbb64ead9362ebb944..19647deecf0a94e6175fc4e1fad8a4a512c4c3df 100644
--- a/commands/bfetch.c
+++ b/commands/bfetch.c
@@ -171,9 +171,17 @@ static inline bool __print_string_list(unsigned *line, const char *key,
 				       struct string_list *sl,
 				       const char *joinstr)
 {
+	char *str;
+
 	if (string_list_empty(sl))
 		return false;
-	print_line(key, "%s", string_list_join(sl, joinstr));
+
+	str = string_list_join(sl, joinstr);
+
+	print_line(key, "%s", str);
+
+	free(str);
+
 	return true;
 }
 #define print_string_list(k, sl, js) __print_string_list(line, k, sl, js)
@@ -309,8 +317,13 @@ static void print_shell_console(unsigned *line)
 
 	for_each_console(console)
 		string_list_add(&sl, console->devfs.name);
-	if (!string_list_empty(&sl))
-		print_line("Consoles", "%s", string_list_join(&sl, " "));
+	if (!string_list_empty(&sl)) {
+		char *str = string_list_join(&sl, " ");
+	
+		print_line("Consoles", "%s", str);
+
+		free(str);
+	}
 
 	string_list_free(&sl);
 }
@@ -332,8 +345,13 @@ static void print_framebuffers(unsigned *line)
 					 info->bits_per_pixel);
 	}
 
-	if (!string_list_empty(&sl))
-		print_line("Framebuffers", "%s", string_list_join(&sl, ", "));
+	if (!string_list_empty(&sl)) {
+		char *str = string_list_join(&sl, ", ");
+
+		print_line("Framebuffers", "%s", str);
+
+		free(str);
+	}
 
 	string_list_free(&sl);
 }

-- 
2.47.3




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/4] fix some memory leaks
  2025-11-07 12:00 [PATCH 0/4] fix some memory leaks Sascha Hauer
                   ` (3 preceding siblings ...)
  2025-11-07 12:00 ` [PATCH 4/4] bfetch: fix memory leak Sascha Hauer
@ 2025-11-10 13:46 ` Sascha Hauer
  4 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2025-11-10 13:46 UTC (permalink / raw)
  To: BAREBOX, Sascha Hauer


On Fri, 07 Nov 2025 13:00:50 +0100, Sascha Hauer wrote:
> This series fixes some memory leaks I stumbled upon
> 
> 

Applied, thanks!

[1/4] tlv: fix memory leak
      https://git.pengutronix.de/cgit/barebox/commit/?id=adedf3ccc4c0 (link may not be stable)
[2/4] of: free of_fixup in of_unregister_fixup()
      https://git.pengutronix.de/cgit/barebox/commit/?id=57562a3ad89d (link may not be stable)
[3/4] bobject: free object name in bobject_del()
      https://git.pengutronix.de/cgit/barebox/commit/?id=1e91e857bf1b (link may not be stable)
[4/4] bfetch: fix memory leak
      https://git.pengutronix.de/cgit/barebox/commit/?id=30626671d567 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/4] bobject: free object name in bobject_del()
  2025-11-07 12:00 ` [PATCH 3/4] bobject: free object name in bobject_del() Sascha Hauer
@ 2025-11-10 13:48   ` Sascha Hauer
  0 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2025-11-10 13:48 UTC (permalink / raw)
  To: BAREBOX

On Fri, Nov 07, 2025 at 01:00:53PM +0100, Sascha Hauer wrote:
> We have bobject_free() which removes all parameters and frees the
> bobject. This works for bobjects which have been allocated with
> bobject_alloc(). struct device has a bobject embedded and is freed
> along with the device, so bobject_free() is inappropriate and
> bobject_del() is called instead. This however doesn't free the bobjects
> name which then leaks when the device is freed.
> 
> To fix this move the freeing of the bobject name to bobject_del(). To
> make this work with CONFIG_PARAMETER disabled drop the static inline
> wrapper.

With this we have to drop the freeing of struct device::name in
free_device_res() as the name is now owned by the bobject.

Fixed up accordingly while applying.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-11-10 13:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-07 12:00 [PATCH 0/4] fix some memory leaks Sascha Hauer
2025-11-07 12:00 ` [PATCH 1/4] tlv: fix memory leak Sascha Hauer
2025-11-07 12:00 ` [PATCH 2/4] of: free of_fixup in of_unregister_fixup() Sascha Hauer
2025-11-07 12:00 ` [PATCH 3/4] bobject: free object name in bobject_del() Sascha Hauer
2025-11-10 13:48   ` Sascha Hauer
2025-11-07 12:00 ` [PATCH 4/4] bfetch: fix memory leak Sascha Hauer
2025-11-10 13:46 ` [PATCH 0/4] fix some memory leaks Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox