Re: [PATCH] Avoid crash on empty menu

[Marco Gerards <mgerards@xs4all.nl>]

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