* [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