[PATCH 0/3] kconfig: Fix expression memory leaks

[PATCH 0/3] kconfig: Fix expression memory leaks

[Ulf Magnusson]

Hello,

This patchset plugs all memory leaks that are due to expressions not being
freed, when parsing the x86 Kconfigs (and likely the other ARCHes too). 336 KB
are leaked in total.

Together with the memory leaks plugged in the parser in
https://lkml.org/lkml/2017/10/8/126, this plugs all memory leaks when parsing
the x86 Kconfigs.

As a reminder, the parsers can be rebuilt like this:

	$ make REGENERATE_PARSERS=1 conf

Here's an easy way to run Valgrind on menuconfig (nothing seems to look at
KERNELVERSION, so just set it to avoid a warning):

	$ ARCH=x86 SRCARCH=x86 KERNELVERSION=4.14.0-rc2 valgrind --leak-check=full scripts/kconfig/mconf Kconfig

Cheers,
Ulf

Ulf Magnusson (3):
  kconfig: Fix automatic menu creation mem leak
  kconfig: Fix expr_free() E_NOT leak
  kconfig: Fix choice symbol expression leak

 scripts/kconfig/expr.c | 2 +-
 scripts/kconfig/menu.c | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH 1/3] kconfig: Fix automatic menu creation mem leak

[Ulf Magnusson]

expr_trans_compare() always allocates and returns a new expression,
giving the following leak outline:

	...
	*Allocate*
	basedep = expr_trans_compare(basedep, E_UNEQUAL, &symbol_no);
	...
	for (menu = parent->next; menu; menu = menu->next) {
		...
		*Copy*
		dep2 = expr_copy(basedep);
		...
		*Free copy*
		expr_free(dep2);
	}
	*basedep lost!*

Fix by freeing 'basedep' after the loop.

Summary from Valgrind on 'menuconfig' (ARCH=x86) before the fix:

	LEAK SUMMARY:
	   definitely lost: 344,376 bytes in 14,349 blocks
	   ...

Summary after the fix:

	LEAK SUMMARY:
	   definitely lost: 44,448 bytes in 1,852 blocks
	   ...

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
 scripts/kconfig/menu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index e935793..749c2bd 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -372,6 +372,7 @@ void menu_finalize(struct menu *parent)
 			menu->parent = parent;
 			last_menu = menu;
 		}
+		expr_free(basedep);
 		if (last_menu) {
 			parent->list = parent->next;
 			parent->next = last_menu->next;
-- 
2.7.4

[PATCH 2/3] kconfig: Fix expr_free() E_NOT leak

[Ulf Magnusson]

Only the E_NOT operand and not the E_NOT node itself was freed, due to
accidentally returning too early in expr_free(). Outline of leak:

	switch (e->type) {
	...
	case E_NOT:
		expr_free(e->left.expr);
		return;
	...
	}
	*Never reached, 'e' leaked*
	free(e);

Fix by changing the 'return' to a 'break'.

Summary from Valgrind on 'menuconfig' (ARCH=x86) before the fix:

	LEAK SUMMARY:
	   definitely lost: 44,448 bytes in 1,852 blocks
	   ...

Summary after the fix:

	LEAK SUMMARY:
	   definitely lost: 1,608 bytes in 67 blocks
	   ...

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
 scripts/kconfig/expr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index cbf4996..ed29bad 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -113,7 +113,7 @@ void expr_free(struct expr *e)
 		break;
 	case E_NOT:
 		expr_free(e->left.expr);
-		return;
+		break;
 	case E_EQUAL:
 	case E_GEQ:
 	case E_GTH:
-- 
2.7.4

[PATCH 3/3] kconfig: Fix choice symbol expression leak

[Ulf Magnusson]

When propagating dependencies from parents after parsing, an expression
node is allocated if the parent symbol is a 'choice'. This node was
never freed.

Outline of leak:

	if (sym && sym_is_choice(sym)) {
		...
		*Allocate (in this case only)*
		parentdep = expr_alloc_symbol(sym);
	} else if (parent->prompt)
		parentdep = parent->prompt->visible.expr;
	else
		parentdep = parent->dep;

	for (menu = parent->list; menu; menu = menu->next) {
		...
		*Copy*
		basedep = expr_alloc_and(expr_copy(parentdep), basedep);
		...
	}
	*parentdep lost if the parent is a choice!*

Fix by freeing 'parentdep' after the loop if the parent symbol is a
choice. Note that this only frees the expression node and not the choice
symbol itself.

Summary from Valgrind on 'menuconfig' (ARCH=x86) before the fix:

	LEAK SUMMARY:
	   definitely lost: 1,608 bytes in 67 blocks
	   ...

Summary after the fix:

	LEAK SUMMARY:
	   definitely lost: 0 bytes in 0 blocks
	   ...

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
 scripts/kconfig/menu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 749c2bd..e0badd9 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -344,6 +344,10 @@ void menu_finalize(struct menu *parent)
 				}
 			}
 		}
+
+		if (sym && sym_is_choice(sym))
+			expr_free(parentdep);
+
 		for (menu = parent->list; menu; menu = menu->next)
 			menu_finalize(menu);
 	} else if (sym) {
-- 
2.7.4

Re: [PATCH 0/3] kconfig: Fix expression memory leaks

[Masahiro Yamada]

2017-10-09 2:35 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> Hello,
>
> This patchset plugs all memory leaks that are due to expressions not being
> freed, when parsing the x86 Kconfigs (and likely the other ARCHes too). 336 KB
> are leaked in total.
>
> Together with the memory leaks plugged in the parser in
> https://lkml.org/lkml/2017/10/8/126, this plugs all memory leaks when parsing
> the x86 Kconfigs.
>
> As a reminder, the parsers can be rebuilt like this:
>
>         $ make REGENERATE_PARSERS=1 conf
>
> Here's an easy way to run Valgrind on menuconfig (nothing seems to look at
> KERNELVERSION, so just set it to avoid a warning):
>
>         $ ARCH=x86 SRCARCH=x86 KERNELVERSION=4.14.0-rc2 valgrind --leak-check=full scripts/kconfig/mconf Kconfig
>
> Cheers,
> Ulf
>
> Ulf Magnusson (3):
>   kconfig: Fix automatic menu creation mem leak
>   kconfig: Fix expr_free() E_NOT leak
>   kconfig: Fix choice symbol expression leak
>

Applied to linux-kbuild/kconfig.  Thanks!



-- 
Best Regards
Masahiro Yamada