public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kconfig: fix memory leak from range properties
@ 2023-11-15  4:16 Masahiro Yamada
  2023-11-16 12:28 ` Boris Kolpackov
  0 siblings, 1 reply; 2+ messages in thread
From: Masahiro Yamada @ 2023-11-15  4:16 UTC (permalink / raw)
  To: linux-kbuild; +Cc: linux-kernel, Masahiro Yamada

Currently, sym_validate_range() duplicates the range string using
xstrdup(), which is overwritten by a subsequent sym_calc_value() call.
It results in a memory leak.

Instead, only the pointer should be copied.

Below is a test case, with a summary from Valgrind.

[Test Kconfig]

  config FOO
          int "foo"
          range 10 20

[Test .config]

  CONFIG_FOO=0

[Before]

  LEAK SUMMARY:
     definitely lost: 3 bytes in 1 blocks
     indirectly lost: 0 bytes in 0 blocks
       possibly lost: 0 bytes in 0 blocks
     still reachable: 17,465 bytes in 21 blocks
          suppressed: 0 bytes in 0 blocks

[After]

  LEAK SUMMARY:
     definitely lost: 0 bytes in 0 blocks
     indirectly lost: 0 bytes in 0 blocks
       possibly lost: 0 bytes in 0 blocks
     still reachable: 17,462 bytes in 20 blocks
          suppressed: 0 bytes in 0 blocks

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kconfig/symbol.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 0572330bf8a7..a76925b46ce6 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -122,9 +122,9 @@ static long long sym_get_range_val(struct symbol *sym, int base)
 static void sym_validate_range(struct symbol *sym)
 {
 	struct property *prop;
+	struct symbol *range_sym;
 	int base;
 	long long val, val2;
-	char str[64];
 
 	switch (sym->type) {
 	case S_INT:
@@ -140,17 +140,15 @@ static void sym_validate_range(struct symbol *sym)
 	if (!prop)
 		return;
 	val = strtoll(sym->curr.val, NULL, base);
-	val2 = sym_get_range_val(prop->expr->left.sym, base);
+	range_sym = prop->expr->left.sym;
+	val2 = sym_get_range_val(range_sym, base);
 	if (val >= val2) {
-		val2 = sym_get_range_val(prop->expr->right.sym, base);
+		range_sym = prop->expr->right.sym;
+		val2 = sym_get_range_val(range_sym, base);
 		if (val <= val2)
 			return;
 	}
-	if (sym->type == S_INT)
-		sprintf(str, "%lld", val2);
-	else
-		sprintf(str, "0x%llx", val2);
-	sym->curr.val = xstrdup(str);
+	sym->curr.val = range_sym->curr.val;
 }
 
 static void sym_set_changed(struct symbol *sym)
-- 
2.40.1


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

* Re: [PATCH] kconfig: fix memory leak from range properties
  2023-11-15  4:16 [PATCH] kconfig: fix memory leak from range properties Masahiro Yamada
@ 2023-11-16 12:28 ` Boris Kolpackov
  0 siblings, 0 replies; 2+ messages in thread
From: Boris Kolpackov @ 2023-11-16 12:28 UTC (permalink / raw)
  To: Masahiro Yamada; +Cc: linux-kbuild, linux-kernel

Masahiro Yamada <masahiroy@kernel.org> writes:

> Currently, sym_validate_range() duplicates the range string using
> xstrdup(), which is overwritten by a subsequent sym_calc_value() call.
> It results in a memory leak.
> 
> [After]
> 
>   LEAK SUMMARY:
>      definitely lost: 0 bytes in 0 blocks
>      indirectly lost: 0 bytes in 0 blocks
>        possibly lost: 0 bytes in 0 blocks
>      still reachable: 17,462 bytes in 20 blocks
>           suppressed: 0 bytes in 0 blocks

FYI, there are quite a few other memory leaks in Kconfig (as evident from
the still reachable value in the above report). I believe I've fixed most
of them in this commit:

https://github.com/build2-packaging/kconfig/commit/cd9910e3636515b2980ce1d37d1984ccfd6b4cb9

In particular, I could load the Linux kernel configuration repeatedly
in a loop without causing any memory leaks or crashes (which were common
due to the state not being reset properly).

I believe the above commit also includes a fix for the sym_validate_range()
leak in question, though the way it's fixed is different. The potential
problem with the proposed fix is that it may be impossible to decide who
should free the shared value.

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

end of thread, other threads:[~2023-11-16 12:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-15  4:16 [PATCH] kconfig: fix memory leak from range properties Masahiro Yamada
2023-11-16 12:28 ` Boris Kolpackov

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