Avoid crash on empty menu
PATCH 1/1 Avoid crash on empty menu
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 <[email protected]>
* normal/menu.c (menu_run): Check for empty menu to avoid crash. (grub_run_menu): Likewise.
normal/menu.c +9 -2
@@ -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 nesif (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);
Quoted 9 lines · Christian Franke
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 :-)
Quoted 39 lines
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 <[email protected]>
*
normal/menu.c(menu_run): Check for empty menu to avoid crash. (grub_run_menu): Likewise.normal/menu.c +5
-1@@ -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 :
normal/menu.c grub_menu_run (grub_menu_t menu, int nes · line 460
Quoted 6 lines
normal/menu.c +3
-0@@ -451,10 +455,13 @@ grub_menu_run (grub_menu_t menu, int nesif (boot_entry <0 )break ; + e =get_entry (menu, boot_entry); +if (! e) +continue ;/* menu is empty */
Please use proper interpunctions for comments.
normal/menu.c grub_menu_run (grub_menu_t menu, int nes · line 467
Quoted 7 lines
normal/menu.c +0
-1grub_cls ();grub_setcursor (1 ); - e =get_entry (menu, boot_entry);grub_printf (" Booting \'%s\'\n\n" , e->title);run_menu_entry (e);
--
Marco
Quoted 9 lines · Marco Gerards
Review
Quoted 5 lines
(quoted diff) +3
-0+ 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 <[email protected]>
* normal/menu.c (run_menu): Check for empty menu to avoid crash. (grub_menu_run): Likewise.
normal/menu.c +9 -2
@@ -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 nesif (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);
Quoted 11 lines · Christian Franke
Quoted 9 lines · Marco Gerards
normal/menu.c grub_menu_run (grub_menu_t menu, int nes
Quoted 5 lines
normal/menu.c +3
-0+ e =get_entry (menu, boot_entry); +if (! e) +continue ;/* menu is empty */ Please use proper interpunctions for comments.
Quoted 8 lines
Fixed.
Christian
2007-11-10 Christian Franke <[email protected]>
*
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
Quoted 7 lines · Marco Gerards
Quoted 5 lines
2007-11-10 Christian Franke <[email protected]>
*
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.
Signature · 5 lines
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 /.)