Re: [PATCH] Avoid crash on empty menu

[Marco Gerards]

Christian Franke <address@hidden> 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  <address@hidden>
>
>       * 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