[syslinux] [PATCH] elflink: fix return from execute()

[Matt Fleming]

On Thu, 2012-07-05 at 20:18 +0200, Sebastian Herbszt wrote:
> Matt Fleming wrote:
> > (Sorry it's taken me so long to review this patch!)
> 
> [snip]
> 
> > I think I see what you're doing here. Did you encounter this problem
> > when loading .c32 modules from, say, vesamenu.c32/menu.c32? Helmut
> > Hullen reported such a problem and I pushed the following fix to my
> > elflink branch,
> > 
> >    commit 316f9636e3958ada609d506deca8db3aef395e54
> >    Author: Matt Fleming <matt.fleming at intel.com>
> >    Date:   Tue Jul 3 09:34:03 2012 +0100
> > 
> >        menu: Supply the command type to execute()
> >    
> >        The old execute() was much more forgiving when passing a COM32 module
> >        as KT_NONE, as the old code for loading a kernel could also handle
> >        COM32 modules. This isn't the case with new_linux_kernel(), and COM32
> >        modules really need to take the create_args_and_load() path in
> >        execute().
> >    
> >        Without this change loading .c32 files from vesamenu.c32/menu.c32
> >        fails.
> >    
> >        Reported-by: Helmut Hullen <Hullen at t-online.de>
> >        Signed-off-by: Matt Fleming <matt.fleming at intel.com>
> > 
> > Which I think is a better way to fix this problem, assuming it *is* the
> > same problem. Historically, the old module loader code could handle .c32
> > files and linux kernels in the same code path. This isn't true anymore
> > and so we need to be more explicit with our file types.
> > 
> > How did you discover this bug? Have you got a test case?
> 
> Test on your elflink branch:
> 
> # cat /tftpboot/pxelinux.cfg/default
> label f
> com32 fancyhello.c32
> 
> PXELINUX 5.00 4.04-pre8-1013-g1f822f8  Copyright (C) 1994-2012 H. Peter Anvin et
>  al
> !PXE entry point found (we hope) at 9C5C:058F via plan A
> UNDI code segment at 9C5C len 0964
> UNDI data segment at 9CF3 len 2CC8
> Getting cached packet  01 02 03
> My IP address seems to be C0A80174 192.168.1.116
> ip=192.168.1.116:192.168.1.99:192.168.1.1:255.255.255.0
> TFTP prefix:
> boot: f
>  *** Hello, World! ***
> > exit
> cmdline = .com32 .com32 fancyhello.c32
> Loading .com32... failed!
> Kernel load failure (insufficient memory?)
> boot: 

Ohhhh... I see what's wrong here. If we successfully lookup the file
type we need to return once we've called execute(). At the moment we'll
fall through the big if statement and take the default case, which is
new_linux_kernel().

I've committed the following patch to my elflink branch,

>From c51d272c7a9d33a10bba63fba359eac9ff7a7f61 Mon Sep 17 00:00:00 2001
From: Matt Fleming <matt.fleming at intel.com>
Date: Fri, 6 Jul 2012 15:36:56 +0100
Subject: [PATCH] ldlinux: Return immediately after we execute() a command

If we successfully lookup the filetype of a command we need to return
once we've called execute(), otherwise we'll also end up calling
new_linux_kernel().

Reported-by: Sebastian Herbszt <herbszt at gmx.de>
Signed-off-by: Matt Fleming <matt.fleming at intel.com>
---
 com32/elflink/ldlinux/execute.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/com32/elflink/ldlinux/execute.c b/com32/elflink/ldlinux/execute.c
index f713eb1..5d128cb 100644
--- a/com32/elflink/ldlinux/execute.c
+++ b/com32/elflink/ldlinux/execute.c
@@ -86,6 +86,7 @@ void execute(const char *cmdline, enum kernel_type type)
 			if (!strcmp(kernel + 1, *pp)) {
 				/* Strip the type specifier and retry */
 				execute(p, type);
+				return;
 			}
 		}
 	}
-- 
1.7.4.4