[PATCH] Avoid crash on empty menu

[PATCH] Avoid crash on empty menu

[Christian Franke]

[-- 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);

Re: [PATCH] Avoid crash on empty menu

[Marco Gerards]

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

Re: [PATCH] Avoid crash on empty menu

[Christian Franke]

[-- 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);

Re: [PATCH] Avoid crash on empty menu

[Marco Gerards]

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

Re: [PATCH] Avoid crash on empty menu

[Robert Millan]

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 /.)