Re: nested functions used by multiboot2 loader corrupt stack

[Bean]

On Jan 17, 2008 7:05 AM, Robert Millan <address@hidden> wrote:
>
> I got pretty confused at this one.  Maybe someone can sort this out.  I'm
> afraid I can't :-(
>
> It seems that at some point when loading multiboot2 images, our stack is
> corrupted for no apparent reason and one of the hooks in our nested function
> calls ends up jumping to the wrong place.
>
> This hangs qemu 0.9.0, but qemu 0.9.1 aborts with "triple fault" message.
>
> I added a few printf calls to trace what's going on, and switched to serial
> terminal so that the output can be captured.  My debugging patch is attached.
> This is the output:
>
> grub_mb2_load_elf: going to call grub_elf32_load using 
> grub_mb2_arch_elf32_hook=0x7ffc72c as hook
> grub_elf32_load: going to call grub_elf32_phdr_iterate using 
> grub_elf32_load_segment=0x7dda4 as hook, and _load_hook=0x7ffc72c as hook's 
> hook
> grub_elf32_phdr_iterate: going to call hook=0x7dda4 using hook_arg=0x7ffc72c 
> as hook
> grub_elf32_load_segment: going to call load_hook=0x7dd9c
> qemu: fatal: triple fault
> EAX=0004be50 EBX=0004bf30 ECX=0008de66 EDX=0007dd2c
> ESI=0004be50 EDI=0007dd9c EBP=0007dd3c ESP=0007dd10
> EIP=0007dda0 EFL=00000002 [-------] CPL=0 II=0 A20=1 SMM=0 HLT=0
>
> This seems to indicate that grub_elf32_phdr_iterate() called its hook, 
> 0x7dda4,
> aka grub_elf32_load_segment() with proper hook_arg parameter = 0x7ffc72c, aka
> grub_mb2_arch_elf32_hook().
>
> When grub_elf32_load_segment() starts, its hook_arg parameter (now known as
> load_hook) has been corrupted and now points at 0x7dd9c.  The other two
> parameters in this function are not tainted, only the third one is.
>
> I'm not sure how to proceed from here.  I really miss a debugger in these
> cases :-(
>
> Also attaching the sample multiboot2 program I used.  I'm not sure of its
> correctness, but nevertheless GRUB shouldn't crash because of incorrect
> images; specially not at this point.

You need to add NESTED_FUNC_ATTR to nested callback function that use
local variable. here is the patch:

diff --git a/kern/elf.c b/kern/elf.c
index b362949..4978a27 100644
--- a/kern/elf.c
+++ b/kern/elf.c
@@ -139,7 +139,7 @@ grub_elf32_load_phdrs (grub_elf_t elf)

 static grub_err_t
 grub_elf32_phdr_iterate (grub_elf_t elf,
-                        int (*hook) (grub_elf_t, Elf32_Phdr *, void *),
+                        int NESTED_FUNC_ATTR (*hook) (grub_elf_t, Elf32_Phdr 
*, void *),
                         void *hook_arg)
 {
   Elf32_Phdr *phdrs;
@@ -219,9 +219,8 @@ grub_elf32_load (grub_elf_t _elf,
grub_elf32_load_hook_t _load_hook,
   grub_size_t load_size = 0;
   grub_err_t err;

-  auto int grub_elf32_load_segment (grub_elf_t elf, Elf32_Phdr *phdr,
-                                   void *hook);
-  int grub_elf32_load_segment (grub_elf_t elf, Elf32_Phdr *phdr, void *hook)
+  auto int NESTED_FUNC_ATTR grub_elf32_load_segment (grub_elf_t elf,
Elf32_Phdr *phdr, void *hook);
+  int NESTED_FUNC_ATTR grub_elf32_load_segment (grub_elf_t elf,
Elf32_Phdr *phdr, void *hook)
   {
     grub_elf32_load_hook_t load_hook = (grub_elf32_load_hook_t) hook;
     grub_addr_t load_addr;


-- 
Bean