* [PATCH] Avoid crash on empty menu
@ 2007-11-07 21:41 Christian Franke
2007-11-09 15:20 ` Marco Gerards
0 siblings, 1 reply; 5+ messages in thread
From: Christian Franke @ 2007-11-07 21:41 UTC (permalink / raw)
To: grub-devel
[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]
If grub.cfg does not contain any valid menuentry statements, an empty
menu is opened.
grub-emu crashes (and real grub behaves "interesting") if the first
entry is selected.
The attached patch adds the missing nullptr checks.
An alternative would be to treat an empty menu as a syntax error in
main.c::read_config_file(), at least if !nested.
During testing, I found the following issues:
- If the file does not exist, read_config_file() produces a memory leak,
because newmenu is allocated first.
- The commands "source FILE" and "configfile FILE" open a nested normal
mode shell (and produce this leak) if the file is missing. An error
message should IMO be printed instead.
- The sequence "c" -> "rescue" -> "normal" appends the same entries to
the existing menu, because the old entry is reused from "menu" data slot.
Thanks for any comment.
Christian
2007-11-07 Christian Franke <franke@computer.org>
* normal/menu.c (menu_run): Check for empty menu to avoid crash.
(grub_run_menu): Likewise.
[-- Attachment #2: grub2-menu-crash.patch --]
[-- Type: text/x-patch, Size: 820 bytes --]
--- grub2.orig/normal/menu.c 2007-08-20 16:35:20.000000000 +0200
+++ grub2/normal/menu.c 2007-11-07 21:57:44.375000000 +0100
@@ -412,7 +412,11 @@ run_menu (grub_menu_t menu, int nested)
goto refresh;
case 'e':
- grub_menu_entry_run (get_entry (menu, first + offset));
+ {
+ grub_menu_entry_t e = get_entry (menu, first + offset);
+ if (e)
+ grub_menu_entry_run (e);
+ }
goto refresh;
default:
@@ -451,10 +455,13 @@ grub_menu_run (grub_menu_t menu, int nes
if (boot_entry < 0)
break;
+ e = get_entry (menu, boot_entry);
+ if (! e)
+ continue; /* menu is empty */
+
grub_cls ();
grub_setcursor (1);
- e = get_entry (menu, boot_entry);
grub_printf (" Booting \'%s\'\n\n", e->title);
run_menu_entry (e);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Avoid crash on empty menu
2007-11-07 21:41 [PATCH] Avoid crash on empty menu Christian Franke
@ 2007-11-09 15:20 ` Marco Gerards
2007-11-10 12:06 ` Christian Franke
0 siblings, 1 reply; 5+ messages in thread
From: Marco Gerards @ 2007-11-09 15:20 UTC (permalink / raw)
To: The development of GRUB 2
Christian Franke <Christian.Franke@t-online.de> writes:
> If grub.cfg does not contain any valid menuentry statements, an empty
> menu is opened.
> grub-emu crashes (and real grub behaves "interesting") if the first
> entry is selected.
>
> The attached patch adds the missing nullptr checks.
>
> An alternative would be to treat an empty menu as a syntax error in
> main.c::read_config_file(), at least if !nested.
Right, perhaps. But for now this is sufficient as it fixes a very
annoying bug :-)
> During testing, I found the following issues:
>
> - If the file does not exist, read_config_file() produces a memory
> leak, because newmenu is allocated first.
>
> - The commands "source FILE" and "configfile FILE" open a nested
> normal mode shell (and produce this leak) if the file is missing. An
> error message should IMO be printed instead.
>
> - The sequence "c" -> "rescue" -> "normal" appends the same entries to
> the existing menu, because the old entry is reused from "menu" data
> slot.
>
> Thanks for any comment.
>
> Christian
>
> 2007-11-07 Christian Franke <franke@computer.org>
>
> * normal/menu.c (menu_run): Check for empty menu to avoid crash.
> (grub_run_menu): Likewise.
>
>
> --- grub2.orig/normal/menu.c 2007-08-20 16:35:20.000000000 +0200
> +++ grub2/normal/menu.c 2007-11-07 21:57:44.375000000 +0100
> @@ -412,7 +412,11 @@ run_menu (grub_menu_t menu, int nested)
> goto refresh;
>
> case 'e':
> - grub_menu_entry_run (get_entry (menu, first + offset));
> + {
> + grub_menu_entry_t e = get_entry (menu, first + offset);
> + if (e)
> + grub_menu_entry_run (e);
> + }
> goto refresh;
>
> default:
> @@ -451,10 +455,13 @@ grub_menu_run (grub_menu_t menu, int nes
> if (boot_entry < 0)
> break;
>
> + e = get_entry (menu, boot_entry);
> + if (! e)
> + continue; /* menu is empty */
Please use proper interpunctions for comments.
> grub_cls ();
> grub_setcursor (1);
>
> - e = get_entry (menu, boot_entry);
> grub_printf (" Booting \'%s\'\n\n", e->title);
>
> run_menu_entry (e);
--
Marco
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Avoid crash on empty menu
2007-11-09 15:20 ` Marco Gerards
@ 2007-11-10 12:06 ` Christian Franke
2007-11-10 15:27 ` Marco Gerards
0 siblings, 1 reply; 5+ messages in thread
From: Christian Franke @ 2007-11-10 12:06 UTC (permalink / raw)
To: The development of GRUB 2
[-- Attachment #1: Type: text/plain, Size: 357 bytes --]
Marco Gerards wrote:
>
>> + e = get_entry (menu, boot_entry);
>> + if (! e)
>> + continue; /* menu is empty */
>>
>
> Please use proper interpunctions for comments.
>
>
Fixed.
Christian
2007-11-10 Christian Franke <franke@computer.org>
* normal/menu.c (run_menu): Check for empty menu to avoid crash.
(grub_menu_run): Likewise.
[-- Attachment #2: grub2-menu-crash-2.patch --]
[-- Type: text/x-patch, Size: 822 bytes --]
--- grub2.orig/normal/menu.c 2007-08-20 16:35:20.000000000 +0200
+++ grub2/normal/menu.c 2007-11-10 12:59:54.953125000 +0100
@@ -412,7 +412,11 @@ run_menu (grub_menu_t menu, int nested)
goto refresh;
case 'e':
- grub_menu_entry_run (get_entry (menu, first + offset));
+ {
+ grub_menu_entry_t e = get_entry (menu, first + offset);
+ if (e)
+ grub_menu_entry_run (e);
+ }
goto refresh;
default:
@@ -451,10 +455,13 @@ grub_menu_run (grub_menu_t menu, int nes
if (boot_entry < 0)
break;
+ e = get_entry (menu, boot_entry);
+ if (! e)
+ continue; /* Menu is empty. */
+
grub_cls ();
grub_setcursor (1);
- e = get_entry (menu, boot_entry);
grub_printf (" Booting \'%s\'\n\n", e->title);
run_menu_entry (e);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Avoid crash on empty menu
2007-11-10 12:06 ` Christian Franke
@ 2007-11-10 15:27 ` Marco Gerards
2007-11-10 20:32 ` Robert Millan
0 siblings, 1 reply; 5+ messages in thread
From: Marco Gerards @ 2007-11-10 15:27 UTC (permalink / raw)
To: The development of GRUB 2
Christian Franke <Christian.Franke@t-online.de> writes:
> Marco Gerards wrote:
>>
>>> + e = get_entry (menu, boot_entry);
>>> + if (! e)
>>> + continue; /* menu is empty */
>>>
>>
>> Please use proper interpunctions for comments.
>>
>>
>
> Fixed.
>
> Christian
>
> 2007-11-10 Christian Franke <franke@computer.org>
>
> * normal/menu.c (run_menu): Check for empty menu to avoid crash.
> (grub_menu_run): Likewise.
This looks ok to me. We can apply this now, it's just a few line.
--
Marco
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Avoid crash on empty menu
2007-11-10 15:27 ` Marco Gerards
@ 2007-11-10 20:32 ` Robert Millan
0 siblings, 0 replies; 5+ messages in thread
From: Robert Millan @ 2007-11-10 20:32 UTC (permalink / raw)
To: The development of GRUB 2
On Sat, Nov 10, 2007 at 04:27:28PM +0100, Marco Gerards wrote:
> >
> > 2007-11-10 Christian Franke <franke@computer.org>
> >
> > * normal/menu.c (run_menu): Check for empty menu to avoid crash.
> > (grub_menu_run): Likewise.
>
> This looks ok to me. We can apply this now, it's just a few line.
Committed.
--
Robert Millan
<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-11-10 20:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-07 21:41 [PATCH] Avoid crash on empty menu Christian Franke
2007-11-09 15:20 ` Marco Gerards
2007-11-10 12:06 ` Christian Franke
2007-11-10 15:27 ` Marco Gerards
2007-11-10 20:32 ` Robert Millan
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.