Re: bugs in loader/i386/pc/multiboot.c

[Tom Bachmann]

Follow-up on part two:
As it turns out, the problem is that the memory where the mmap is stored 
to is never allocated at all. The problem is confined to the elf64 code. 
I attach a patch (against svn HEAD I believe) that fixes this issue 
(essentially by inserting a snippet of elf32 code). It also contains the 
two comment-outs (still commented out and not deleted because I don't 
know what the code there is supposed to do). The patch is under the same 
license as the file it applies to, of course. If anyone cares, here's a 
minimal changelog:

loader/i386/pc/multiboot.c

(grub_multiboot_load_elf64): Initialize grub_multiboot_payload_size, 
grub_multiboot_payload_dest and payload properly.

(grub_multiboot): Don't overwrite entry variable after being initialized.

Tom Bachmann wrote:
> 2) Reading the mmap doesn't work.
> Now this could be a qemu artifact (would still be nice to fix), but even 
> with the two above-mentioned lines removed the code doesn't seem to 
> work. I still have to uncomment lines 466-468 which call 
> grub_fill_multiboot_mmap. This call doesn't seem to return. Indeed my 
> introspection printfs suggest that the inner hook of that function is 
> called three times, but adding printfs to grub_machine_map_iterate (in 
> kern/i386/pc/mmap.c I believe) doesn't seem to work (just crashes grub?) 
> and so I had to stop looking around here.

--
-ness-

Index: loader/i386/pc/multiboot.c
===================================================================
--- loader/i386/pc/multiboot.c  (Revision 1946)
+++ loader/i386/pc/multiboot.c  (Arbeitskopie)
@@ -223,6 +223,7 @@
   char *phdr_base;
   grub_addr_t physical_entry_addr = 0;
   int i;
+  int lowest_segment = 0, highest_segment = 0;
 
   if (ehdr->e_ident[EI_CLASS] != ELFCLASS64)
     return grub_error (GRUB_ERR_UNKNOWN_OS, "invalid ELF class");
@@ -252,6 +253,21 @@
   phdr_base = (char *) buffer + ehdr->e_phoff;
 #define phdr(i)                        ((Elf64_Phdr *) (phdr_base + (i) * 
ehdr->e_phentsize))
 
+  for (i = 0; i < ehdr->e_phnum; i++)
+    if (phdr(i)->p_type == PT_LOAD && phdr(i)->p_filesz != 0)
+      {
+       if (phdr(i)->p_paddr < phdr(lowest_segment)->p_paddr)
+         lowest_segment = i;
+       if (phdr(i)->p_paddr > phdr(highest_segment)->p_paddr)
+         highest_segment = i;
+      }
+  grub_multiboot_payload_size += (phdr(highest_segment)->p_paddr + 
phdr(highest_segment)->p_memsz) - phdr(lowest_segment)->p_paddr;
+  grub_multiboot_payload_dest = phdr(lowest_segment)->p_paddr;
+
+  playground = grub_malloc (RELOCATOR_SIZEOF(forward) + 
grub_multiboot_payload_size + RELOCATOR_SIZEOF(backward));
+  if (! playground)
+    return grub_errno;
+
   /* Load every loadable segment in memory.  */
   for (i = 0; i < ehdr->e_phnum; i++)
     {
@@ -475,13 +491,13 @@
   if (grub_multiboot_payload_dest >= grub_multiboot_payload_orig)
     {
       grub_memmove (playground, &grub_multiboot_forward_relocator, 
RELOCATOR_SIZEOF(forward));
-      entry = (grub_addr_t) playground;
+      //entry = (grub_addr_t) playground;
     }
   else
     {
       grub_memmove ((char *) (grub_multiboot_payload_orig + 
grub_multiboot_payload_size),
                    &grub_multiboot_backward_relocator, 
RELOCATOR_SIZEOF(backward));
-      entry = (grub_addr_t) grub_multiboot_payload_orig + 
grub_multiboot_payload_size;
+      //entry = (grub_addr_t) grub_multiboot_payload_orig + 
grub_multiboot_payload_size;
     }
   
   grub_dprintf ("multiboot_loader", "dest=%p, size=0x%x, entry_offset=0x%x\n",