nconfig: prevent segfault on empty menu
PATCH 1/1 nconfig: prevent segfault on empty menu
Hello,
i found and fixed an NULL-dereference bug in nconf tool.
How to reproduce:
1. $ make nconfig
2. disable "Kernel hacking -> Debug Filesystem"
3. go to "General setup -> GCOV-based kernel profiling" and hit F2 it should segfault
Fix: i have added some checks for "struct menu*" to be NULL before it get dereferenced
Regards,
Andrej Gelenberg
From 82be343a388a02477ffb0d464e1f2810c61a1fda Mon Sep 17 00:00:00 2001
From: Andrej Gelenberg <[email protected]>
Date: Sun, 10 Jul 2011 03:44:50 +0200
Subject: [PATCH] nconfig: prevent segfault on empty menu
how to reproduce:
1. $ make nconfig
2. disable "Kernel hacking -> Debug Filesystem"
3. go to "General setup -> GCOV-based kernel profiling" and hit F2 it should segfault
Fix: i have added some checks for "struct menu*" to be NULL bevor it get dereferenced
--- scripts/kconfig/menu.c | 18 ++++++++++-------- 1 files changed, 10 insertions(+), 8 deletions(-)
scripts/kconfig/menu.c +10 -8
@@ -425,7 +425,7 @@ void menu_finalize(struct menu *parent)bool menu_has_prompt (struct menu *menu) { -if (!menu->prompt) +if ((!menu) || (!menu->prompt))return false;return true; }
@@ -436,7 +436,7 @@ bool menu_is_visible(struct menu *menu)struct symbol *sym;tristate visible; -if (!menu->prompt) +if ((!menu) || !menu->prompt)return false;if (menu->visibility) {
@@ -470,10 +470,12 @@ bool menu_is_visible(struct menu *menu)const char *menu_get_prompt (struct menu *menu) { -if (menu->prompt) -return menu->prompt->text; -else if (menu->sym) -return menu->sym->name; +if (menu) { +if (menu->prompt) +return menu->prompt->text; +else if (menu->sym) +return menu->sym->name; + }return NULL ; }
@@ -496,12 +498,12 @@ struct menu *menu_get_parent_menu(struct menu *menu)bool menu_has_help (struct menu *menu) { -return menu->help !=NULL ; +return menu && (menu->help !=NULL ); }const char *menu_get_help (struct menu *menu) { -if (menu->help) +if (menu && menu->help)return menu->help;else return "" ;
Signature · 1 line
1.7.6
Hi,
Quoted 4 lines · Andrej Gelenberg
Hello,
i found and fixed an NULL-dereference bug in nconf tool.
Quoted 9 lines
How to reproduce:
1. $ make nconfig
2. disable "Kernel hacking -> Debug Filesystem"
3. go to "General setup -> GCOV-based kernel profiling" and hit F2 it should segfaultFix: i have added some checks for "struct menu*" to be NULL before it get dereferenced
I am not a huge fan of this. The frontend should be careful not to try to request the visibility or the prompt of an invalid menu. Following this point of view, I would rather see assertion added than a test for NULL, and fix the frontend appropriately to catch those case. Said otherwise, there is no reason to apply a fix on the backend for an nconfig-only issue.
- Arnaud
Quoted 3 lines
Regards,
Andrej Gelenberg
PATCH 1/2 kconfig/nconf: use the generic menu_get_ext_help()
nconf is the only front-end which does not use this helper, but prefer to copy/paste the code. The test wrt. menu validity added in this version of the code is bogus anyway as an invalid menu will get dereferenced a few line below by calling menu_get_prompt().
For now, convert nconf to use menu_get_ext_help(), as do every other front-end. We will deals with menu validity checks properly in a separate commit.
--- scripts/kconfig/nconf.c | 14 +------------- 1 files changed, 1 insertions(+), 13 deletions(-)
scripts/kconfig/nconf.c +1 -13
@@ -1223,19 +1223,7 @@ static void conf_message_callback(const char *fmt, va_list ap)static void show_help (struct menu *menu) {struct gstr help =str_new (); - -if (menu && menu->sym &&menu_has_help (menu)) { -if (menu->sym->name) { -str_printf (&help,"%s%s:\n\n" ,CONFIG_ , menu->sym->name); -str_append (&help,_ (menu_get_help (menu))); -str_append (&help,"\n" ); -get_symbol_str (&help, menu->sym); - }else { -str_append (&help,_ (menu_get_help (menu))); - } - }else { -str_append (&help, nohelp_text); - } +menu_get_ext_help (menu, &help);show_scroll_win (main_window,_ (menu_get_prompt (menu)),str_get (&help));str_free (&help); }
Signature · 1 line
1.7.3.4.574.g608b.dirty
PATCH 2/2 kconfig/nconf: prevent segfault on empty menu
nconf does not check the validity of the current menu when help is requested (with either <F2>, '?' or 'h'). This leads to a NULL pointer dereference when an empty menu is encountered.
The following reduced testcase exposes the problem:
config DEP
bool
menu "FOO"
config BAR
bool "BAR"
depends on DEP
endmenu
Issue will happen when entering menu "FOO" and requesting help.
nconf is the only front-end which do not filter the validity of the current menu. Such filter can not really happen beforehand as other key which does not deals with the current menu might be entered by the user, so just bails out earlier if we encounter an invalid menu.
--- scripts/kconfig/nconf.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
scripts/kconfig/nconf.c +6 -1
@@ -1222,7 +1222,12 @@ static void conf_message_callback(const char *fmt, va_list ap)static void show_help (struct menu *menu) { -struct gstr help =str_new (); +struct gstr help; + +if (!menu) +return ; + + help =str_new ();menu_get_ext_help (menu, &help);show_scroll_win (main_window,_ (menu_get_prompt (menu)),str_get (&help));str_free (&help);
Signature · 1 line
1.7.3.4.574.g608b.dirty
Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
Hello.
Quoted 15 lines · Arnaud Lacombe
nconf does not check the validity of the current menu when help is requested (with either <F2>, '?' or 'h'). This leads to a NULL pointer dereference when an empty menu is encountered.
The following reduced testcase exposes the problem:
config DEP
boolmenu "FOO"
config BAR
bool "BAR"
depends on DEP
Quoted 8 lines
endmenu
Issue will happen when entering menu "FOO" and requesting help.
nconf is the only front-end which do not filter the validity of the current menu. Such filter can not really happen beforehand as other key which does not deals with the current menu might be entered by the user, so just bails out earlier if we encounter an invalid menu.
I do not believe the correct solution is silently ignoring the user's key presses. I think the correct behavior would be to display a nohelp_text window.
something like this (untested):show_scroll_win(main_window, nohelp_text);
The other patches in the series seem gr8.
Cheers.
Quoted 30 lines
Cc Nir Tzachar <[email protected]>Cc Andrej Gelenberg <[email protected]>Reported-by Andrej Gelenberg <[email protected]>Signed-off-by Arnaud Lacombe <[email protected]>---
scripts/kconfig/nconf.c | 7 ++++++-1 files changed, 6 insertions(+), 1 deletions(-)scripts/kconfig/nconf.c +6
-2@@ -1222,7 +1222,12 @@ static void conf_message_callback(const char *fmt, va_list ap) static void show_help(struct menu *menu) { - struct gstr help = str_new(); + struct gstr help; + + if (!menu) + return; + + help = str_new(); menu_get_ext_help(menu, &help); show_scroll_win(main_window, _(menu_get_prompt(menu)), str_get(&help)); str_free(&help); --1.7.3.4.574.g608b.dirty
Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
Hi,
Quoted 27 lines · Nir Tzachar
Hello.
Quoted 15 lines · Arnaud Lacombe
nconf does not check the validity of the current menu when help is requested (with either <F2>, '?' or 'h'). This leads to a NULL pointer dereference when an empty menu is encountered.
The following reduced testcase exposes the problem:
config DEP
boolmenu "FOO"
config BAR
bool "BAR"
depends on DEPQuoted 8 lines
endmenu
Issue will happen when entering menu "FOO" and requesting help.
nconf is the only front-end which do not filter the validity of the current menu. Such filter can not really happen beforehand as other key which does not deals with the current menu might be entered by the user, so just bails out earlier if we encounter an invalid menu.
Quoted 7 lines
I do not believe the correct solution is silently ignoring the user's key presses. I think the correct behavior would be to display a
nohelp_textwindow.something like this (untested):
show_scroll_win(main_window, nohelp_text);
This would be:
show_scroll_win(main_window, _(menu_get_prompt(menu)), nohelp_text);
but we would fall back on the original issue as requesting the prompt of an invalid menu does not make sense, so the menu_get_prompt(menu) would still have to be replaced with something valid. It would be nice to come up with a solution valid at least for nconf and mconf. Currently mconf just silently discard the key pressed.
To some extend I do not think any frontend should display a menu which has no visible child, but that may require more changes and affect performance...
- Arnaud
Quoted 35 lines
The other patches in the series seem gr8.
Cheers.
Quoted 30 lines
Cc Nir Tzachar <[email protected]>Cc Andrej Gelenberg <[email protected]>Reported-by Andrej Gelenberg <[email protected]>Signed-off-by Arnaud Lacombe <[email protected]>---
scripts/kconfig/nconf.c | 7 ++++++-1 files changed, 6 insertions(+), 1 deletions(-)scripts/kconfig/nconf.c +6
-2@@ -1222,7 +1222,12 @@ static void conf_message_callback(const char *fmt, va_list ap) static void show_help(struct menu *menu) { - struct gstr help = str_new(); + struct gstr help; + + if (!menu) + return; + + help = str_new(); menu_get_ext_help(menu, &help); show_scroll_win(main_window, _(menu_get_prompt(menu)), str_get(&help)); str_free(&help); --1.7.3.4.574.g608b.dirty
Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
Quoted 34 lines · Arnaud Lacombe
Hi,
Quoted 31 lines · Nir Tzachar
Hello.
Quoted 15 lines · Arnaud Lacombe
nconf does not check the validity of the current menu when help is requested (with either <F2>, '?' or 'h'). This leads to a NULL pointer dereference when an empty menu is encountered.
The following reduced testcase exposes the problem:
config DEP
boolmenu "FOO"
config BAR
bool "BAR"
depends on DEPQuoted 8 lines
endmenu
Issue will happen when entering menu "FOO" and requesting help.
nconf is the only front-end which do not filter the validity of the current menu. Such filter can not really happen beforehand as other key which does not deals with the current menu might be entered by the user, so just bails out earlier if we encounter an invalid menu.
I do not believe the correct solution is silently ignoring the user's key presses. I think the correct behavior would be to display a
nohelp_textwindow.
Quoted 6 lines
Quoted 3 lines
something like this (untested):
show_scroll_win(main_window, nohelp_text);This would be:
show_scroll_win(main_window, _(menu_get_prompt(menu)), nohelp_text);
show_scroll_win(main_window, _("No help available"), nohelp_text);
would also work, and have none of the complications.
Quoted 48 lines
but we would fall back on the original issue as requesting the prompt of an invalid menu does not make sense, so the
menu_get_prompt(menu)would still have to be replaced with something valid. It would be nice to come up with a solution valid at least for nconf and mconf. Currently mconf just silently discard the key pressed.To some extend I do not think any frontend should display a menu which has no visible child, but that may require more changes and affect performance...
- Arnaud
Quoted 35 lines
The other patches in the series seem gr8.
Cheers.
Quoted 30 lines
Cc Nir Tzachar <[email protected]>Cc Andrej Gelenberg <[email protected]>Reported-by Andrej Gelenberg <[email protected]>Signed-off-by Arnaud Lacombe <[email protected]>---
scripts/kconfig/nconf.c | 7 ++++++-1 files changed, 6 insertions(+), 1 deletions(-)scripts/kconfig/nconf.c +6
-2@@ -1222,7 +1222,12 @@ static void conf_message_callback(const char *fmt, va_list ap) static void show_help(struct menu *menu) { - struct gstr help = str_new(); + struct gstr help; + + if (!menu) + return; + + help = str_new(); menu_get_ext_help(menu, &help); show_scroll_win(main_window, _(menu_get_prompt(menu)), str_get(&help)); str_free(&help); --1.7.3.4.574.g608b.dirty
Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
In reply to Nir TzacharHello,
it show already "There is no help available for this option." message on F2. Only caption could be better as just "(null)". Besides ignore the key is much better as segfault.
Regards,
Andrej Gelenberg
Quoted 68 lines · Nir Tzachar
Hello.
Quoted 15 lines · Arnaud Lacombe
nconf does not check the validity of the current menu when help is requested (with either <F2>, '?' or 'h'). This leads to a NULL pointer dereference when an empty menu is encountered.
The following reduced testcase exposes the problem:
config DEP
boolmenu "FOO"
config BAR
bool "BAR"
depends on DEPQuoted 8 lines
endmenu
Issue will happen when entering menu "FOO" and requesting help.
nconf is the only front-end which do not filter the validity of the current menu. Such filter can not really happen beforehand as other key which does not deals with the current menu might be entered by the user, so just bails out earlier if we encounter an invalid menu.
I do not believe the correct solution is silently ignoring the user's key presses. I think the correct behavior would be to display a
nohelp_textwindow.something like this (untested):
show_scroll_win(main_window, nohelp_text);The other patches in the series seem gr8.
Cheers.
Quoted 30 lines
Cc Nir Tzachar <[email protected]>Cc Andrej Gelenberg <[email protected]>Reported-by Andrej Gelenberg <[email protected]>Signed-off-by Arnaud Lacombe <[email protected]>--- scripts/kconfig/nconf.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)scripts/kconfig/nconf.c +6
-2@@ -1222,7 +1222,12 @@ static void conf_message_callback(const char *fmt, va_list ap)static void show_help (struct menu *menu) { -struct gstr help =str_new (); +struct gstr help; + +if (!menu) +return ; + + help =str_new ();menu_get_ext_help (menu, &help);show_scroll_win (main_window,_ (menu_get_prompt (menu)),str_get (&help));str_free (&help); --1.7.3.4.574.g608b.dirty
Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
Quoted 5 lines · Andrej Gelenberg
Hello,
it show already "There is no help available for this option." message on F2. Only caption could be better as just "(null)". Besides ignore the key is much better as segfault.
I agree. Then just add a proper caption and thats it.
Cheers.
Quoted 74 lines
Regards
Andrej GelenbergQuoted 68 lines · Nir Tzachar
Hello.
Quoted 15 lines · Arnaud Lacombe
nconf does not check the validity of the current menu when help is requested (with either <F2>, '?' or 'h'). This leads to a NULL pointer dereference when an empty menu is encountered.
The following reduced testcase exposes the problem:
config DEP
boolmenu "FOO"
config BAR
bool "BAR"
depends on DEPQuoted 8 lines
endmenu
Issue will happen when entering menu "FOO" and requesting help.
nconf is the only front-end which do not filter the validity of the current menu. Such filter can not really happen beforehand as other key which does not deals with the current menu might be entered by the user, so just bails out earlier if we encounter an invalid menu.
I do not believe the correct solution is silently ignoring the user's key presses. I think the correct behavior would be to display a
nohelp_textwindow.something like this (untested):
show_scroll_win(main_window, nohelp_text);The other patches in the series seem gr8.
Cheers.
Quoted 30 lines
Cc Nir Tzachar <[email protected]>Cc Andrej Gelenberg <[email protected]>Reported-by Andrej Gelenberg <[email protected]>Signed-off-by Arnaud Lacombe <[email protected]>---
scripts/kconfig/nconf.c | 7 ++++++-1 files changed, 6 insertions(+), 1 deletions(-)scripts/kconfig/nconf.c +6
-2@@ -1222,7 +1222,12 @@ static void conf_message_callback(const char *fmt, va_list ap) static void show_help(struct menu *menu) { - struct gstr help = str_new(); + struct gstr help; + + if (!menu) + return; + + help = str_new(); menu_get_ext_help(menu, &help); show_scroll_win(main_window, _(menu_get_prompt(menu)), str_get(&help)); str_free(&help); --1.7.3.4.574.g608b.dirty
Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
Hi,
here is one more patch, which make empty title instead of "(null)".
Regards,
Andrej Gelenberg
Quoted 4 lines · Nir Tzachar
Quoted 2 lines · Andrej Gelenberg
Hello,
Quoted 10 lines
Quoted 3 lines
it show already "There is no help available for this option." message on F2. Only caption could be better as just "(null)". Besides ignore the key is much better as segfault.
I agree. Then just add a proper caption and thats it.
Cheers.
Quoted 2 lines
Regards
Andrej Gelenberg
From e5fa0e9a705c2e2092ba9c2879e85f8bef12f7f8 Mon Sep 17 00:00:00 2001
From: Andrej Gelenberg <[email protected]>
Date: Tue, 12 Jul 2011 18:36:50 +0200
Subject: [PATCH] nconfig: menu_get_prompt() return "", if no promt
Help message box has "" as titled instead of "(null)" on empty menu in nconf.
--- scripts/kconfig/menu.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
scripts/kconfig/menu.c +1 -1
@@ -476,7 +476,7 @@ const char *menu_get_prompt(struct menu *menu)else if (menu->sym)return menu->sym->name; } -return NULL ; +return "" ; }struct menu *menu_get_root_menu (struct menu *menu)
Signature · 1 line
1.7.6
Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
Hi,
Quoted 4 lines · Andrej Gelenberg
Hi,
here is one more patch, which make empty title instead of "(null)".
No. You are breaking front-ends which either properly use the API and make use menu_has_prompt() before getting the prompt with menu_get_prompt(), or test for NULL pointer and act accordingly. Btw, by doing this, you potentially breaking nconf too, and there is rarely any need to modify the back-end to fix display issue in the front-end.
- Arnaud
ps: could you please send inlined patched, that a real pain to review. thanks.
Quoted 19 lines
Regards,
Andrej GelenbergQuoted 14 lines · Nir Tzachar
Quoted 5 lines · Andrej Gelenberg
Hello,
it show already "There is no help available for this option." message on F2. Only caption could be better as just "(null)". Besides ignore the key is much better as segfault.
I agree. Then just add a proper caption and thats it.
Cheers.
Quoted 2 lines
Regards
Andrej Gelenberg
Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
In reply to Arnaud LacombeQuoted 24 lines · Arnaud Lacombe
nconf does not check the validity of the current menu when help is requested (with either <F2>, '?' or 'h'). This leads to a NULL pointer dereference when an empty menu is encountered.
The following reduced testcase exposes the problem:
config DEP
boolmenu "FOO"
config BAR
bool "BAR"
depends on DEPendmenu
Issue will happen when entering menu "FOO" and requesting help.
nconf is the only front-end which do not filter the validity of the current menu. Such filter can not really happen beforehand as other key which does not deals with the current menu might be entered by the user, so just bails out earlier if we encounter an invalid menu.
Quoted 4 lines
Cc Nir Tzachar <[email protected]>Cc Andrej Gelenberg <[email protected]>Reported-by Andrej Gelenberg <[email protected]>Signed-off-by Arnaud Lacombe <[email protected]>
Thanks, applied both patches to kbuild-2.6.git#kconfig.
Michal
Re: [PATCH 1/2] kconfig/nconf: use the generic menu_get_ext_help()
In reply to Arnaud LacombeHi,
Quoted 31 lines · Arnaud Lacombe
nconf is the only front-end which does not use this helper, but prefer to copy/paste the code. The test wrt. menu validity added in this version of the code is bogus anyway as an invalid menu will get dereferenced a few line below by calling
menu_get_prompt().For now, convert nconf to use
menu_get_ext_help(), as do every other front-end. We will deals with menu validity checks properly in a separate commit.Cc Nir Tzachar <[email protected]>Cc Andrej Gelenberg <[email protected]>Signed-off-by Arnaud Lacombe <[email protected]>---
scripts/kconfig/nconf.c | 14 +-------------1 files changed, 1 insertions(+), 13 deletions(-)scripts/kconfig/nconf.c +0
-7@@ -1223,19 +1223,7 @@ static void conf_message_callback(const char *fmt, va_list ap) static void show_help(struct menu *menu) { struct gstr help = str_new(); - - if (menu && menu->sym && menu_has_help(menu)) { - if (menu->sym->name) { - str_printf(&help, "%s%s:\n\n", CONFIG_, menu->sym->name); - str_append(&help, _(menu_get_help(menu))); - str_append(&help, "\n"); - get_symbol_str(&help, menu->sym);
scripts/kconfig/nconf.c static void conf_message_callback(const char *fmt, va_list ap) · line 1229
Quoted 10 lines
scripts/kconfig/nconf.c +1
-6- } else { - str_append(&help, _(menu_get_help(menu))); - } - } else { - str_append(&help, nohelp_text); - } + menu_get_ext_help(menu, &help); show_scroll_win(main_window, _(menu_get_prompt(menu)), str_get(&help)); str_free(&help); }
Michal, ping ?
- Arnaud
Quoted 4 lines
--
1.7.3.4.574.g608b.dirty
Re: [PATCH 1/2] kconfig/nconf: use the generic menu_get_ext_help()
Hi,
Quoted 37 lines · Arnaud Lacombe
Hi,
Quoted 34 lines · Arnaud Lacombe
nconf is the only front-end which does not use this helper, but prefer to copy/paste the code. The test wrt. menu validity added in this version of the code is bogus anyway as an invalid menu will get dereferenced a few line below by calling
menu_get_prompt().For now, convert nconf to use
menu_get_ext_help(), as do every other front-end. We will deals with menu validity checks properly in a separate commit.Cc Nir Tzachar <[email protected]>Cc Andrej Gelenberg <[email protected]>Signed-off-by Arnaud Lacombe <[email protected]>---
scripts/kconfig/nconf.c | 14 +-------------1 files changed, 1 insertions(+), 13 deletions(-)scripts/kconfig/nconf.c +0
-11@@ -1223,19 +1223,7 @@ static void conf_message_callback(const char *fmt, va_list ap) static void show_help(struct menu *menu) { struct gstr help = str_new(); - - if (menu && menu->sym && menu_has_help(menu)) { - if (menu->sym->name) { - str_printf(&help, "%s%s:\n\n", CONFIG_, menu->sym->name); - str_append(&help, _(menu_get_help(menu))); - str_append(&help, "\n"); - get_symbol_str(&help, menu->sym); - } else { - str_append(&help, _(menu_get_help(menu))); - } - } else {
Quoted 9 lines
scripts/kconfig/nconf.c static void conf_message_callback(const char *fmt, va_list ap) · line 1229
Quoted 7 lines
scripts/kconfig/nconf.c +1
-2- str_append(&help, nohelp_text); - } + menu_get_ext_help(menu, &help); show_scroll_win(main_window, _(menu_get_prompt(menu)), str_get(&help)); str_free(&help); }Michal, ping ?
Sorry for my last 3 ping, the patches in question have been merged, I did not pay attention :-).
- Arnaud