linux-kernel - [PATCH] kconfig: Prevent segfault when getting filename

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240618185609.4096399-1-elsk@google.com>
Date: Tue, 18 Jun 2024 18:56:09 +0000
From: Yifan Hong <elsk@...gle.com>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: Yifan Hong <elsk@...gle.com>, kernel-team@...roid.com, linux-kbuild@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: [PATCH] kconfig: Prevent segfault when getting filename

... and lineno in recursive checks.

If the following snippet is found in Kconfig:

config FOO
        tristate
        depends on BAR
        select BAR
        help
          foo

... without BAR defined; then if one runs
`make tinyconfig`, there is a segfault.

  Kconfig:34:error: recursive dependency detected!
  Kconfig:34:	symbol FOO depends on BAR
  make[4]: *** [scripts/kconfig/Makefile:85: allnoconfig] Segmentation fault

This is because of the following. BAR is
a fake entry created by sym_lookup() with prop
being NULL. In the recursive check, there is a
NULL check for prop to fall back to
stack->sym->prop if stack->prop is NULL. However,
in this case, stack->sym points to the fake BAR
entry created by sym_lookup(), so prop is still
NULL. prop was then referenced without additional
NULL checks, causing segfault.

Similarly, menu is also accessed without NULL
checks. However, sym_lookup() creates entry
that is not a choice, so technically it shouldn't
fall into the state where menu is NULL for
choices. But I mechnically apply the NULL check
anyways for completeness.

This mechnical patch avoids the segfault. The
above snippet produces the following error with
this patch:

  Kconfig:34:error: recursive dependency detected!
  Kconfig:34:	symbol FOO depends on BAR
  ???:-1:	symbol BAR is selected by FOO

That being said, I am not sure if it is the right
fix conceptually and in functionality.

Signed-off-by: Yifan Hong <elsk@...gle.com>
---
 scripts/kconfig/symbol.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 8df0a75f40b9..72ab4f274289 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -1045,6 +1045,8 @@ static void sym_check_print_recursive(struct symbol *last_sym)
 	struct menu *menu = NULL;
 	struct property *prop;
 	struct dep_stack cv_stack;
+	const char *filename = NULL;
+	int lineno = 0;
 
 	if (sym_is_choice_value(last_sym)) {
 		dep_stack_insert(&cv_stack, last_sym);
@@ -1060,6 +1062,10 @@ static void sym_check_print_recursive(struct symbol *last_sym)
 	}
 
 	for (; stack; stack = stack->next) {
+		filename = "???";
+		lineno = 0;
+		menu = NULL;
+
 		sym = stack->sym;
 		next_sym = stack->next ? stack->next->sym : last_sym;
 		prop = stack->prop;
@@ -1073,45 +1079,52 @@ static void sym_check_print_recursive(struct symbol *last_sym)
 				if (prop->menu)
 					break;
 			}
+			if (menu) {
+				filename = menu->filename;
+				lineno = menu->lineno;
+			}
+		} else if (prop) {
+			filename = prop->filename;
+			lineno = prop->lineno;
 		}
 		if (stack->sym == last_sym)
 			fprintf(stderr, "%s:%d:error: recursive dependency detected!\n",
-				prop->filename, prop->lineno);
+				filename, lineno);
 
 		if (sym_is_choice(sym)) {
 			fprintf(stderr, "%s:%d:\tchoice %s contains symbol %s\n",
-				menu->filename, menu->lineno,
+				filename, lineno,
 				sym->name ? sym->name : "<choice>",
 				next_sym->name ? next_sym->name : "<choice>");
 		} else if (sym_is_choice_value(sym)) {
 			fprintf(stderr, "%s:%d:\tsymbol %s is part of choice %s\n",
-				menu->filename, menu->lineno,
+				filename, lineno,
 				sym->name ? sym->name : "<choice>",
 				next_sym->name ? next_sym->name : "<choice>");
 		} else if (stack->expr == &sym->dir_dep.expr) {
 			fprintf(stderr, "%s:%d:\tsymbol %s depends on %s\n",
-				prop->filename, prop->lineno,
+				filename, lineno,
 				sym->name ? sym->name : "<choice>",
 				next_sym->name ? next_sym->name : "<choice>");
 		} else if (stack->expr == &sym->rev_dep.expr) {
 			fprintf(stderr, "%s:%d:\tsymbol %s is selected by %s\n",
-				prop->filename, prop->lineno,
+				filename, lineno,
 				sym->name ? sym->name : "<choice>",
 				next_sym->name ? next_sym->name : "<choice>");
 		} else if (stack->expr == &sym->implied.expr) {
 			fprintf(stderr, "%s:%d:\tsymbol %s is implied by %s\n",
-				prop->filename, prop->lineno,
+				filename, lineno,
 				sym->name ? sym->name : "<choice>",
 				next_sym->name ? next_sym->name : "<choice>");
 		} else if (stack->expr) {
 			fprintf(stderr, "%s:%d:\tsymbol %s %s value contains %s\n",
-				prop->filename, prop->lineno,
+				filename, lineno,
 				sym->name ? sym->name : "<choice>",
 				prop_get_type_name(prop->type),
 				next_sym->name ? next_sym->name : "<choice>");
 		} else {
 			fprintf(stderr, "%s:%d:\tsymbol %s %s is visible depending on %s\n",
-				prop->filename, prop->lineno,
+				filename, lineno,
 				sym->name ? sym->name : "<choice>",
 				prop_get_type_name(prop->type),
 				next_sym->name ? next_sym->name : "<choice>");
-- 
2.45.2.627.g7a2c4fd464-goog