All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nconfig: prevent segfault on empty menu
@ 2011-07-10  2:04 ` Andrej Gelenberg
  0 siblings, 0 replies; 16+ messages in thread
From: Andrej Gelenberg @ 2011-07-10  2:04 UTC (permalink / raw)
  To: linux-kernel, linux-kbuild

[-- Attachment #1: Type: text/plain, Size: 351 bytes --]

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

[-- Attachment #2: 0001-nconfig-prevent-segfault-on-empty-menu.patch --]
[-- Type: text/plain, Size: 1882 bytes --]

>From 82be343a388a02477ffb0d464e1f2810c61a1fda Mon Sep 17 00:00:00 2001
From: Andrej Gelenberg <andrej.gelenberg@udo.edu>
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

Signed-off-by: Andrej Gelenberg <andrej.gelenberg@udo.edu>
---
 scripts/kconfig/menu.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 5fdf10d..6a09cc4 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -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 "";
-- 
1.7.6


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

* [PATCH] nconfig: prevent segfault on empty menu
@ 2011-07-10  2:04 ` Andrej Gelenberg
  0 siblings, 0 replies; 16+ messages in thread
From: Andrej Gelenberg @ 2011-07-10  2:04 UTC (permalink / raw)
  To: linux-kernel, linux-kbuild

[-- Attachment #1: Type: text/plain, Size: 351 bytes --]

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

[-- Attachment #2: 0001-nconfig-prevent-segfault-on-empty-menu.patch --]
[-- Type: text/plain, Size: 1881 bytes --]

From 82be343a388a02477ffb0d464e1f2810c61a1fda Mon Sep 17 00:00:00 2001
From: Andrej Gelenberg <andrej.gelenberg@udo.edu>
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

Signed-off-by: Andrej Gelenberg <andrej.gelenberg@udo.edu>
---
 scripts/kconfig/menu.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 5fdf10d..6a09cc4 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -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 "";
-- 
1.7.6


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

* Re: [PATCH] nconfig: prevent segfault on empty menu
  2011-07-10  2:04 ` Andrej Gelenberg
  (?)
@ 2011-07-10  6:02 ` Arnaud Lacombe
  2011-07-10  7:27   ` [PATCH 1/2] kconfig/nconf: use the generic menu_get_ext_help() Arnaud Lacombe
  -1 siblings, 1 reply; 16+ messages in thread
From: Arnaud Lacombe @ 2011-07-10  6:02 UTC (permalink / raw)
  To: Andrej Gelenberg; +Cc: linux-kernel, linux-kbuild

Hi,

On Sat, Jul 9, 2011 at 10:04 PM, Andrej Gelenberg
<andrej.gelenberg@udo.edu> wrote:
> 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
>
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

> Regards,
> Andrej Gelenberg
>

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

* [PATCH 1/2] kconfig/nconf: use the generic menu_get_ext_help()
  2011-07-10  6:02 ` Arnaud Lacombe
@ 2011-07-10  7:27   ` Arnaud Lacombe
  2011-07-10  7:27     ` [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu Arnaud Lacombe
  2011-07-18 18:11     ` [PATCH 1/2] kconfig/nconf: use the generic menu_get_ext_help() Arnaud Lacombe
  0 siblings, 2 replies; 16+ messages in thread
From: Arnaud Lacombe @ 2011-07-10  7:27 UTC (permalink / raw)
  To: linux-kbuild, Michal Marek
  Cc: linux-kernel, Arnaud Lacombe, Nir Tzachar, Andrej Gelenberg

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 <nir.tzachar@gmail.com>
Cc: Andrej Gelenberg <andrej.gelenberg@udo.edu>
Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
---
 scripts/kconfig/nconf.c |   14 +-------------
 1 files changed, 1 insertions(+), 13 deletions(-)

diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index da9f5c4..24fc79a 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -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);
 }
-- 
1.7.3.4.574.g608b.dirty


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

* [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
  2011-07-10  7:27   ` [PATCH 1/2] kconfig/nconf: use the generic menu_get_ext_help() Arnaud Lacombe
@ 2011-07-10  7:27     ` Arnaud Lacombe
  2011-07-10 10:32       ` Nir Tzachar
  2011-07-13 11:49       ` Michal Marek
  2011-07-18 18:11     ` [PATCH 1/2] kconfig/nconf: use the generic menu_get_ext_help() Arnaud Lacombe
  1 sibling, 2 replies; 16+ messages in thread
From: Arnaud Lacombe @ 2011-07-10  7:27 UTC (permalink / raw)
  To: linux-kbuild, Michal Marek
  Cc: linux-kernel, Arnaud Lacombe, Nir Tzachar, Andrej Gelenberg

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.

Cc: Nir Tzachar <nir.tzachar@gmail.com>
Cc: Andrej Gelenberg <andrej.gelenberg@udo.edu>
Reported-by: Andrej Gelenberg <andrej.gelenberg@udo.edu>
Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
---
 scripts/kconfig/nconf.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 24fc79a..eb9e49d 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -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


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

* Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
  2011-07-10  7:27     ` [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu Arnaud Lacombe
@ 2011-07-10 10:32       ` Nir Tzachar
  2011-07-10 16:18         ` Arnaud Lacombe
  2011-07-11  8:51         ` Andrej Gelenberg
  2011-07-13 11:49       ` Michal Marek
  1 sibling, 2 replies; 16+ messages in thread
From: Nir Tzachar @ 2011-07-10 10:32 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: linux-kbuild, Michal Marek, linux-kernel, Andrej Gelenberg

Hello.

On Sun, Jul 10, 2011 at 10:27 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> 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.

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.

> Cc: Nir Tzachar <nir.tzachar@gmail.com>
> Cc: Andrej Gelenberg <andrej.gelenberg@udo.edu>
> Reported-by: Andrej Gelenberg <andrej.gelenberg@udo.edu>
> Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
> ---
>  scripts/kconfig/nconf.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
> index 24fc79a..eb9e49d 100644
> --- a/scripts/kconfig/nconf.c
> +++ b/scripts/kconfig/nconf.c
> @@ -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
>
>

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

* Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
  2011-07-10 10:32       ` Nir Tzachar
@ 2011-07-10 16:18         ` Arnaud Lacombe
  2011-07-11  6:19           ` Nir Tzachar
  2011-07-11  8:51         ` Andrej Gelenberg
  1 sibling, 1 reply; 16+ messages in thread
From: Arnaud Lacombe @ 2011-07-10 16:18 UTC (permalink / raw)
  To: Nir Tzachar; +Cc: linux-kbuild, Michal Marek, linux-kernel, Andrej Gelenberg

Hi,

On Sun, Jul 10, 2011 at 6:32 AM, Nir Tzachar <nir.tzachar@gmail.com> wrote:
> Hello.
>
> On Sun, Jul 10, 2011 at 10:27 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
>> 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.
>
> 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);
>
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

> The other patches in the series seem gr8.
>
> Cheers.
>
>> Cc: Nir Tzachar <nir.tzachar@gmail.com>
>> Cc: Andrej Gelenberg <andrej.gelenberg@udo.edu>
>> Reported-by: Andrej Gelenberg <andrej.gelenberg@udo.edu>
>> Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
>> ---
>>  scripts/kconfig/nconf.c |    7 ++++++-
>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
>> index 24fc79a..eb9e49d 100644
>> --- a/scripts/kconfig/nconf.c
>> +++ b/scripts/kconfig/nconf.c
>> @@ -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
>>
>>
>

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

* Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
  2011-07-10 16:18         ` Arnaud Lacombe
@ 2011-07-11  6:19           ` Nir Tzachar
  0 siblings, 0 replies; 16+ messages in thread
From: Nir Tzachar @ 2011-07-11  6:19 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: linux-kbuild, Michal Marek, linux-kernel, Andrej Gelenberg

On Sun, Jul 10, 2011 at 7:18 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> Hi,
>
> On Sun, Jul 10, 2011 at 6:32 AM, Nir Tzachar <nir.tzachar@gmail.com> wrote:
>> Hello.
>>
>> On Sun, Jul 10, 2011 at 10:27 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
>>> 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.
>>
>> 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);
>>
> 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.

> 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
>
>> The other patches in the series seem gr8.
>>
>> Cheers.
>>
>>> Cc: Nir Tzachar <nir.tzachar@gmail.com>
>>> Cc: Andrej Gelenberg <andrej.gelenberg@udo.edu>
>>> Reported-by: Andrej Gelenberg <andrej.gelenberg@udo.edu>
>>> Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
>>> ---
>>>  scripts/kconfig/nconf.c |    7 ++++++-
>>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
>>> index 24fc79a..eb9e49d 100644
>>> --- a/scripts/kconfig/nconf.c
>>> +++ b/scripts/kconfig/nconf.c
>>> @@ -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
>>>
>>>
>>
>

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

* Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
  2011-07-10 10:32       ` Nir Tzachar
  2011-07-10 16:18         ` Arnaud Lacombe
@ 2011-07-11  8:51         ` Andrej Gelenberg
  2011-07-11  9:50           ` Nir Tzachar
  1 sibling, 1 reply; 16+ messages in thread
From: Andrej Gelenberg @ 2011-07-11  8:51 UTC (permalink / raw)
  To: Nir Tzachar, linux-kbuild, mmarek, linux-kernel

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.

Regards,
Andrej Gelenberg

On 07/10/2011 12:32 PM, Nir Tzachar wrote:
> Hello.
> 
> On Sun, Jul 10, 2011 at 10:27 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
>> 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.
> 
> 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.
> 
>> Cc: Nir Tzachar <nir.tzachar@gmail.com>
>> Cc: Andrej Gelenberg <andrej.gelenberg@udo.edu>
>> Reported-by: Andrej Gelenberg <andrej.gelenberg@udo.edu>
>> Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
>> ---
>>  scripts/kconfig/nconf.c |    7 ++++++-
>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
>> index 24fc79a..eb9e49d 100644
>> --- a/scripts/kconfig/nconf.c
>> +++ b/scripts/kconfig/nconf.c
>> @@ -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
>>
>>


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

* Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
  2011-07-11  8:51         ` Andrej Gelenberg
@ 2011-07-11  9:50           ` Nir Tzachar
  2011-07-12 20:17               ` Andrej Gelenberg
  0 siblings, 1 reply; 16+ messages in thread
From: Nir Tzachar @ 2011-07-11  9:50 UTC (permalink / raw)
  To: Andrej Gelenberg; +Cc: linux-kbuild, mmarek, linux-kernel

On Mon, Jul 11, 2011 at 11:51 AM, Andrej Gelenberg
<andrej.gelenberg@udo.edu> wrote:
> 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.

> Regards
> Andrej Gelenberg
>
> On 07/10/2011 12:32 PM, Nir Tzachar wrote:
>> Hello.
>>
>> On Sun, Jul 10, 2011 at 10:27 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
>>> 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.
>>
>> 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.
>>
>>> Cc: Nir Tzachar <nir.tzachar@gmail.com>
>>> Cc: Andrej Gelenberg <andrej.gelenberg@udo.edu>
>>> Reported-by: Andrej Gelenberg <andrej.gelenberg@udo.edu>
>>> Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
>>> ---
>>>  scripts/kconfig/nconf.c |    7 ++++++-
>>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
>>> index 24fc79a..eb9e49d 100644
>>> --- a/scripts/kconfig/nconf.c
>>> +++ b/scripts/kconfig/nconf.c
>>> @@ -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
>>>
>>>
>
>

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

* Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
  2011-07-11  9:50           ` Nir Tzachar
@ 2011-07-12 20:17               ` Andrej Gelenberg
  0 siblings, 0 replies; 16+ messages in thread
From: Andrej Gelenberg @ 2011-07-12 20:17 UTC (permalink / raw)
  To: Nir Tzachar; +Cc: linux-kbuild, mmarek, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

Hi,

here is one more patch, which make empty title instead of "(null)".

Regards,
Andrej Gelenberg

On 07/11/2011 11:50 AM, Nir Tzachar wrote:
> On Mon, Jul 11, 2011 at 11:51 AM, Andrej Gelenberg
> <andrej.gelenberg@udo.edu> wrote:
>> 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.
> 
>> Regards
>> Andrej Gelenberg

[-- Attachment #2: 0001-nconfig-menu_get_prompt-return-if-no-promt.patch --]
[-- Type: text/plain, Size: 822 bytes --]

>From e5fa0e9a705c2e2092ba9c2879e85f8bef12f7f8 Mon Sep 17 00:00:00 2001
From: Andrej Gelenberg <andrej.gelenberg@udo.edu>
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.

Signed-off-by: Andrej Gelenberg <andrej.gelenberg@udo.edu>
---
 scripts/kconfig/menu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 6a09cc4..0783d8e 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -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)
-- 
1.7.6


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

* Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
@ 2011-07-12 20:17               ` Andrej Gelenberg
  0 siblings, 0 replies; 16+ messages in thread
From: Andrej Gelenberg @ 2011-07-12 20:17 UTC (permalink / raw)
  To: Nir Tzachar; +Cc: linux-kbuild, mmarek, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

Hi,

here is one more patch, which make empty title instead of "(null)".

Regards,
Andrej Gelenberg

On 07/11/2011 11:50 AM, Nir Tzachar wrote:
> On Mon, Jul 11, 2011 at 11:51 AM, Andrej Gelenberg
> <andrej.gelenberg@udo.edu> wrote:
>> 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.
> 
>> Regards
>> Andrej Gelenberg

[-- Attachment #2: 0001-nconfig-menu_get_prompt-return-if-no-promt.patch --]
[-- Type: text/plain, Size: 821 bytes --]

From e5fa0e9a705c2e2092ba9c2879e85f8bef12f7f8 Mon Sep 17 00:00:00 2001
From: Andrej Gelenberg <andrej.gelenberg@udo.edu>
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.

Signed-off-by: Andrej Gelenberg <andrej.gelenberg@udo.edu>
---
 scripts/kconfig/menu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 6a09cc4..0783d8e 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -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)
-- 
1.7.6


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

* Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
  2011-07-12 20:17               ` Andrej Gelenberg
  (?)
@ 2011-07-12 20:49               ` Arnaud Lacombe
  -1 siblings, 0 replies; 16+ messages in thread
From: Arnaud Lacombe @ 2011-07-12 20:49 UTC (permalink / raw)
  To: Andrej Gelenberg; +Cc: Nir Tzachar, linux-kbuild, mmarek, linux-kernel

Hi,

On Tue, Jul 12, 2011 at 4:17 PM, Andrej Gelenberg
<andrej.gelenberg@udo.edu> wrote:
> 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.

> Regards,
> Andrej Gelenberg
>
> On 07/11/2011 11:50 AM, Nir Tzachar wrote:
>> On Mon, Jul 11, 2011 at 11:51 AM, Andrej Gelenberg
>> <andrej.gelenberg@udo.edu> wrote:
>>> 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.
>>
>>> Regards
>>> Andrej Gelenberg
>

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

* Re: [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu
  2011-07-10  7:27     ` [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu Arnaud Lacombe
  2011-07-10 10:32       ` Nir Tzachar
@ 2011-07-13 11:49       ` Michal Marek
  1 sibling, 0 replies; 16+ messages in thread
From: Michal Marek @ 2011-07-13 11:49 UTC (permalink / raw)
  To: Arnaud Lacombe; +Cc: linux-kbuild, linux-kernel, Nir Tzachar, Andrej Gelenberg

On Sun, Jul 10, 2011 at 03:27:05AM -0400, Arnaud Lacombe wrote:
> 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.
> 
> Cc: Nir Tzachar <nir.tzachar@gmail.com>
> Cc: Andrej Gelenberg <andrej.gelenberg@udo.edu>
> Reported-by: Andrej Gelenberg <andrej.gelenberg@udo.edu>
> Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>

Thanks, applied both patches to kbuild-2.6.git#kconfig.

Michal

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

* Re: [PATCH 1/2] kconfig/nconf: use the generic menu_get_ext_help()
  2011-07-10  7:27   ` [PATCH 1/2] kconfig/nconf: use the generic menu_get_ext_help() Arnaud Lacombe
  2011-07-10  7:27     ` [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu Arnaud Lacombe
@ 2011-07-18 18:11     ` Arnaud Lacombe
  2011-07-18 19:02       ` Arnaud Lacombe
  1 sibling, 1 reply; 16+ messages in thread
From: Arnaud Lacombe @ 2011-07-18 18:11 UTC (permalink / raw)
  To: linux-kbuild, Michal Marek
  Cc: linux-kernel, Arnaud Lacombe, Nir Tzachar, Andrej Gelenberg

Hi,

On Sun, Jul 10, 2011 at 3:27 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> 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 <nir.tzachar@gmail.com>
> Cc: Andrej Gelenberg <andrej.gelenberg@udo.edu>
> Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
> ---
>  scripts/kconfig/nconf.c |   14 +-------------
>  1 files changed, 1 insertions(+), 13 deletions(-)
>
> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
> index da9f5c4..24fc79a 100644
> --- a/scripts/kconfig/nconf.c
> +++ b/scripts/kconfig/nconf.c
> @@ -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);
>  }
>
Michal, ping ?

 - Arnaud

> --
> 1.7.3.4.574.g608b.dirty
>
>

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

* Re: [PATCH 1/2] kconfig/nconf: use the generic menu_get_ext_help()
  2011-07-18 18:11     ` [PATCH 1/2] kconfig/nconf: use the generic menu_get_ext_help() Arnaud Lacombe
@ 2011-07-18 19:02       ` Arnaud Lacombe
  0 siblings, 0 replies; 16+ messages in thread
From: Arnaud Lacombe @ 2011-07-18 19:02 UTC (permalink / raw)
  To: linux-kbuild, Michal Marek
  Cc: linux-kernel, Arnaud Lacombe, Nir Tzachar, Andrej Gelenberg

Hi,

On Mon, Jul 18, 2011 at 2:11 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> Hi,
>
> On Sun, Jul 10, 2011 at 3:27 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
>> 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 <nir.tzachar@gmail.com>
>> Cc: Andrej Gelenberg <andrej.gelenberg@udo.edu>
>> Signed-off-by: Arnaud Lacombe <lacombar@gmail.com>
>> ---
>>  scripts/kconfig/nconf.c |   14 +-------------
>>  1 files changed, 1 insertions(+), 13 deletions(-)
>>
>> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
>> index da9f5c4..24fc79a 100644
>> --- a/scripts/kconfig/nconf.c
>> +++ b/scripts/kconfig/nconf.c
>> @@ -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);
>>  }
>>
> Michal, ping ?
>
Sorry for my last 3 ping, the patches in question have been merged, I
did not pay attention :-).

 - Arnaud

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

end of thread, other threads:[~2011-07-18 19:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-10  2:04 [PATCH] nconfig: prevent segfault on empty menu Andrej Gelenberg
2011-07-10  2:04 ` Andrej Gelenberg
2011-07-10  6:02 ` Arnaud Lacombe
2011-07-10  7:27   ` [PATCH 1/2] kconfig/nconf: use the generic menu_get_ext_help() Arnaud Lacombe
2011-07-10  7:27     ` [PATCH 2/2] kconfig/nconf: prevent segfault on empty menu Arnaud Lacombe
2011-07-10 10:32       ` Nir Tzachar
2011-07-10 16:18         ` Arnaud Lacombe
2011-07-11  6:19           ` Nir Tzachar
2011-07-11  8:51         ` Andrej Gelenberg
2011-07-11  9:50           ` Nir Tzachar
2011-07-12 20:17             ` Andrej Gelenberg
2011-07-12 20:17               ` Andrej Gelenberg
2011-07-12 20:49               ` Arnaud Lacombe
2011-07-13 11:49       ` Michal Marek
2011-07-18 18:11     ` [PATCH 1/2] kconfig/nconf: use the generic menu_get_ext_help() Arnaud Lacombe
2011-07-18 19:02       ` Arnaud Lacombe

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.