All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ulf Magnusson <ulfalizer@gmail.com>
To: yann.morin.1998@free.fr, linux-kbuild@vger.kernel.org
Cc: sam@ravnborg.org, zippel@linux-m68k.org,
	nicolas.pitre@linaro.org, mmarek@suse.com, dirk@gouders.net,
	yamada.masahiro@socionext.com, lacombar@gmail.com,
	JBeulich@suse.com, linux-kernel@vger.kernel.org,
	Ulf Magnusson <ulfalizer@gmail.com>
Subject: [PATCH 3/3] kconfig: Clean up modules handling and fix crash
Date: Thu,  5 Oct 2017 14:01:15 +0200	[thread overview]
Message-ID: <1507204875-5021-4-git-send-email-ulfalizer@gmail.com> (raw)
In-Reply-To: <1507204875-5021-1-git-send-email-ulfalizer@gmail.com>

Kconfig currently doesn't handle 'm' appearing in a Kconfig file before
the modules symbol is defined (the symbol with 'option modules'). The
problem is the following code, which runs during parsing:

	/* change 'm' into 'm' && MODULES */
	if (e->left.sym == &symbol_mod)
		return expr_alloc_and(e, expr_alloc_symbol(modules_sym));

If the modules symbol has not yet been defined, modules_sym is NULL,
giving an invalid expression.

Here is a test file where both BEFORE_1 and BEFORE_2 trigger a segfault.
If the modules symbol is removed, all symbols trigger segfaults.

	config BEFORE_1
		def_tristate y if m

	if m
	config BEFORE_2
		def_tristate y
	endif

	config MODULES
		def_bool y
		option modules

	config AFTER_1
		def_tristate y if m

	if m
	config AFTER_2
		def_tristate y
	endif

Fix the issue by rewriting 'm' in menu_finalize() instead. This function
runs after parsing and is the proper place to do it. The following
existing code in conf_parse() in zconf.y ensures that the modules symbol
exists at that point:

	if (!modules_sym)
		modules_sym = sym_find( "n" );

	...

	menu_finalize(&rootmenu);

The following tests were done to ensure no functional changes for
configurations that don't reference 'm' before the modules symbol:

	- zconfdump(stdout) was run with ARCH=x86 and ARCH=arm before
	  and after the change and verified to produce identical output.
	  This function prints all symbols, choices, and menus together
	  with their properties and their dependency expressions. A
	  rewritten 'm' appears as 'm && MODULES'.

	  A small annoyance is that the assert(len != 0) in xfwrite()
	  needs to be disabled in order to use zconfdump(), because it
	  chokes on e.g. 'default ""'.

	- The Kconfiglib test suite was run to indirectly verify that
	  alldefconfig, allyesconfig, allnoconfig, and all defconfigs in
	  the kernel still generate the same final .config.

	- Valgrind was used to check for memory errors and (new) memory
	  leaks.

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

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 94f192de..a7396e4 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -110,7 +110,7 @@ static struct expr *rewrite_m(struct expr *e)
 
 void menu_add_dep(struct expr *dep)
 {
-	current_entry->dep = expr_alloc_and(current_entry->dep, rewrite_m(dep));
+	current_entry->dep = expr_alloc_and(current_entry->dep, dep);
 }
 
 void menu_set_type(int type)
@@ -135,7 +135,7 @@ static struct property *menu_add_prop(enum prop_type type, char *prompt, struct
 
 	prop->menu = current_entry;
 	prop->expr = expr;
-	prop->visible.expr = rewrite_m(dep);
+	prop->visible.expr = dep;
 
 	if (prompt) {
 		if (isspace(*prompt)) {
@@ -330,7 +330,8 @@ void menu_finalize(struct menu *parent)
 			 * Propagate parent dependencies to the child menu
 			 * node, also rewriting and simplifying expressions
 			 */
-			basedep = expr_transform(menu->dep);
+			basedep = rewrite_m(menu->dep);
+			basedep = expr_transform(basedep);
 			basedep = expr_alloc_and(expr_copy(parentdep), basedep);
 			basedep = expr_eliminate_dups(basedep);
 			menu->dep = basedep;
@@ -373,7 +374,8 @@ void menu_finalize(struct menu *parent)
 				 * property's condition, rewriting and
 				 * simplifying expressions at the same time
 				 */
-				dep = expr_transform(prop->visible.expr);
+				dep = rewrite_m(prop->visible.expr);
+				dep = expr_transform(dep);
 				dep = expr_alloc_and(expr_copy(basedep), dep);
 				dep = expr_eliminate_dups(dep);
 				if (menu->sym && menu->sym->type != S_TRISTATE)
-- 
2.7.4


  parent reply	other threads:[~2017-10-05 12:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-05 12:01 [PATCH 0/3] kconfig: Make 'm' safe before modules symbol is defined Ulf Magnusson
2017-10-05 12:01 ` [PATCH 1/3] kconfig: Rename menu_check_dep() to rewrite_m() Ulf Magnusson
2017-10-05 12:01 ` [PATCH 2/3] kconfig: Clarify expression rewriting Ulf Magnusson
2017-10-05 12:01 ` Ulf Magnusson [this message]
2017-12-14 23:27 ` [PATCH 0/3] kconfig: Make 'm' safe before modules symbol is defined Masahiro Yamada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1507204875-5021-4-git-send-email-ulfalizer@gmail.com \
    --to=ulfalizer@gmail.com \
    --cc=JBeulich@suse.com \
    --cc=dirk@gouders.net \
    --cc=lacombar@gmail.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mmarek@suse.com \
    --cc=nicolas.pitre@linaro.org \
    --cc=sam@ravnborg.org \
    --cc=yamada.masahiro@socionext.com \
    --cc=yann.morin.1998@free.fr \
    --cc=zippel@linux-m68k.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.