| Message ID | 1310282825-14509-2-git-send-email-lacombar@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >>> >>> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >> >> -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >>> >>> > > -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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(-)