[PATCH v2] multiboot: Add support for program headers at "strange" locations

[Jacob Paul]

Currently, grub_multiboot_load_elf() will throw an error if
the program header is at a too high offset (since it would be outside
the allocated buffer space of MULTIBOOT_SEARCH bytes). It is understandable
why it does this, since 99.99% of all cases e_phoff has a value of 0x34 
or 0x40.
So, to be able to support a (uncommon) circumstance like this, the 
program header
can be loaded into a separate buffer if needed, and then freed at the end.
The only downside to this is that grub_multiboot_load_elf() must at 
every return
statement check if phdr_base is a separate buffer and if so de-allocate it.

Signed-off-by: Jacob Paul <address@hidden>
---

Changes in v2:
- Add line breaks to commit message as suggested by John Paul Adrian 
Glaubitz (for some reason the line breaks were removed by my MUA).
- Removed whitespace.

 grub-core/loader/multiboot_elfxx.c | 86 +++++++++++++++++++++++++-----
 1 file changed, 74 insertions(+), 12 deletions(-)

diff --git a/grub-core/loader/multiboot_elfxx.c 
b/grub-core/loader/multiboot_elfxx.c
index 70cd1db51..f6aa156a0 100644
--- a/grub-core/loader/multiboot_elfxx.c
+++ b/grub-core/loader/multiboot_elfxx.c
@@ -75,12 +75,37 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t 
*mld)
   if (ehdr->e_type != ET_EXEC && ehdr->e_type != ET_DYN)
     return grub_error (GRUB_ERR_UNKNOWN_OS, N_("this ELF file is not 
of the right type"));

-  /* FIXME: Should we support program headers at strange locations?  */
+  /* Program header outside of the allocated buffer space. Weird, but 
just work around it by allocating a new buffer for it */
   if (ehdr->e_phoff + (grub_uint32_t) ehdr->e_phnum * 
ehdr->e_phentsize > MULTIBOOT_SEARCH)
-    return grub_error (GRUB_ERR_BAD_OS, "program header at a too high 
offset");
+    {
+      phdr_base = grub_malloc ((grub_size_t) ehdr->e_phnum * 
ehdr->e_phentsize);
+      if (!phdr_base)
+    return grub_errno;
+
+      if (grub_file_seek (mld->file, (grub_off_t) ehdr->e_phoff) == 
(grub_off_t) -1)
+        {
+          grub_free(phdr_base);
+          return grub_error (GRUB_ERR_BAD_OS, "invalid program header 
offset");
+        }
+
+      if (grub_file_read (mld->file, phdr_base, (grub_size_t) 
ehdr->e_phnum * ehdr->e_phentsize)
+      != (grub_ssize_t) ehdr->e_phnum * ehdr->e_phentsize)
+        {
+          grub_free (phdr_base);
+          if (!grub_errno)
+            grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of 
file %s"), mld->filename);
+          return grub_errno;
+        }
+    }
+  else
+    {
+      phdr_base = (char *) mld->buffer + ehdr->e_phoff;
+    }

-  phdr_base = (char *) mld->buffer + ehdr->e_phoff;
 #define phdr(i)                        ((Elf_Phdr *) (phdr_base + (i) * 
ehdr->e_phentsize))
+/* FIXME: find a better way to de-allocate phdr_base if an error occurs.
+    Putting this in front of every exit is probably not the prettiest 
solution */
+#define phdr_free() if ((grub_size_t) phdr_base != (grub_size_t) 
mld->buffer + ehdr->e_phoff) grub_free(phdr_base);

   mld->link_base_addr = ~0;

@@ -94,7 +119,10 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t 
*mld)

 #ifdef MULTIBOOT_LOAD_ELF64
   if (highest_load >= 0x100000000)
-    return grub_error (GRUB_ERR_BAD_OS, "segment crosses 4 GiB border");
+    {
+      phdr_free();
+      return grub_error (GRUB_ERR_BAD_OS, "segment crosses 4 GiB border");
+    }
 #endif

   if (mld->relocatable)
@@ -107,7 +135,10 @@ CONCAT(grub_multiboot_load_elf, XX) 
(mbi_load_data_t *mld)
                    mld->avoid_efi_boot_services);

       if (load_size > mld->max_addr || mld->min_addr > mld->max_addr - 
load_size)
-	return grub_error (GRUB_ERR_BAD_OS, "invalid min/max address and/or 
load size");
+        {
+          phdr_free();
+          return grub_error (GRUB_ERR_BAD_OS, "invalid min/max address 
and/or load size");
+        }

       err = grub_relocator_alloc_chunk_align (GRUB_MULTIBOOT 
(relocator), &ch,
                                              mld->min_addr, mld->max_addr - 
load_size,
@@ -117,6 +148,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t 
*mld)
       if (err)
         {
           grub_dprintf ("multiboot_loader", "Cannot allocate memory 
for OS image\n");
+          phdr_free();
           return err;
         }

@@ -152,6 +184,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t 
*mld)
              if (err)
                {
 		  grub_dprintf ("multiboot_loader", "Cannot allocate memory for OS 
image\n");
+      phdr_free();
                  return err;
                }

@@ -162,7 +195,10 @@ CONCAT(grub_multiboot_load_elf, XX) 
(mbi_load_data_t *mld)
            {
              if (grub_file_seek (mld->file, (grub_off_t) phdr(i)->p_offset)
                  == (grub_off_t) -1)
-               return grub_errno;
+    {
+      phdr_free();
+                 return grub_errno;
+    }

 	      if (grub_file_read (mld->file, (grub_uint8_t *) source + 
load_offset, phdr(i)->p_filesz)
                  != (grub_ssize_t) phdr(i)->p_filesz)
@@ -170,6 +206,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t 
*mld)
                  if (!grub_errno)
 		    grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of file 
%s"),
                                mld->filename);
+      phdr_free();
                  return grub_errno;
                }
            }
@@ -191,19 +228,28 @@ CONCAT(grub_multiboot_load_elf, XX) 
(mbi_load_data_t *mld)
   /* We still in 32-bit mode.  */
   if ((ehdr->e_entry - phdr(i)->p_vaddr)
       + phdr(i)->p_paddr < 0xffffffff80000000ULL)
-    return grub_error (GRUB_ERR_BAD_OS, "invalid entry point for ELF64");
+    {
+      phdr_free();
+      return grub_error (GRUB_ERR_BAD_OS, "invalid entry point for ELF64");
+    }
 # else
   /* We still in 32-bit mode.  */
   if ((ehdr->e_entry - phdr(i)->p_vaddr)
       + phdr(i)->p_paddr > 0xffffffff)
-    return grub_error (GRUB_ERR_BAD_OS, "invalid entry point for ELF64");
+    {
+      phdr_free();
+      return grub_error (GRUB_ERR_BAD_OS, "invalid entry point for ELF64");
+    }
 # endif
 #endif
        break;
       }

   if (i == ehdr->e_phnum)
-    return grub_error (GRUB_ERR_BAD_OS, "entry point isn't in a segment");
+    {
+      phdr_free();
+      return grub_error (GRUB_ERR_BAD_OS, "entry point isn't in a 
segment");
+    }

 #if defined (__i386__) || defined (__x86_64__)

@@ -219,11 +265,15 @@ CONCAT(grub_multiboot_load_elf, XX) 
(mbi_load_data_t *mld)

       shdr = grub_malloc ((grub_uint32_t) ehdr->e_shnum * 
ehdr->e_shentsize);
       if (!shdr)
-       return grub_errno;
+  {
+    phdr_free();
+         return grub_errno;
+  }

       if (grub_file_seek (mld->file, ehdr->e_shoff) == (grub_off_t) -1)
        {
          grub_free (shdr);
+    phdr_free();
          return grub_errno;
        }

@@ -233,6 +283,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t 
*mld)
          if (!grub_errno)
            grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of file 
%s"),
                        mld->filename);
+    phdr_free();
          return grub_errno;
        }

@@ -244,7 +295,10 @@ CONCAT(grub_multiboot_load_elf, XX) 
(mbi_load_data_t *mld)
          grub_addr_t target;

 	  if (mld->mbi_ver >= 2 && (sh->sh_type == SHT_REL || sh->sh_type == 
SHT_RELA))
-	    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, "ELF files with 
relocs are not supported yet");
+      {
+        phdr_free();
+	      return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, "ELF files with 
relocs are not supported yet");
+      }

          /* This section is a loaded section,
             so we don't care.  */
@@ -263,13 +317,17 @@ CONCAT(grub_multiboot_load_elf, XX) 
(mbi_load_data_t *mld)
          if (err)
            {
              grub_dprintf ("multiboot_loader", "Error loading shdr %d\n", i);
+        phdr_free();
              return err;
            }
          src = get_virtual_current_address (ch);
          target = get_physical_target_address (ch);

          if (grub_file_seek (mld->file, sh->sh_offset) == (grub_off_t) -1)
-           return grub_errno;
+      {
+        phdr_free();
+             return grub_errno;
+      }

           if (grub_file_read (mld->file, src, sh->sh_size)
               != (grub_ssize_t) sh->sh_size)
@@ -277,6 +335,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t 
*mld)
              if (!grub_errno)
                grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of file 
%s"),
                            mld->filename);
+        phdr_free();
              return grub_errno;
            }
          sh->sh_addr = target;
@@ -285,7 +344,10 @@ CONCAT(grub_multiboot_load_elf, XX) 
(mbi_load_data_t *mld)
                                    ehdr->e_shstrndx, shdr);
     }

+  phdr_free();
+
 #undef phdr
+#undef phdr_free

   return grub_errno;
 }
--
2.26.2