elf: Validate number of elf program header table entries

In bsdXX.c and multiboot_elfxx.c, e_phnum is used to obtain the number of
program header table entries, but it wasn't being checked if the value was
there.

According to the elf(5) manual page,
"If the number of entries in the program header table is larger than or equal to
PN_XNUM (0xffff), this member holds PN_XNUM (0xffff) and the real number of
entries in the program header table is held in the sh_info member of the
initial entry in section header table.  Otherwise, the sh_info member of the
initial entry contains the value zero."

Since this check wasn't being made, grub_elfXX_get_phnum() is being added to
elfXX.c to make this check and use e_phnum if it doesn't have PN_XNUM as a
value, else use sh_info. We also need to make sure e_phnum isn't greater than
PN_XNUM and sh_info isn't less than PN_XNUM.

Note that even though elf.c and elfXX.c are located in grub-core/kern, they are
compiled as modules and don't need the EXPORT_FUNC() macro to define the functions
in elf.h.

Also, changed casts of phnum to match variables being set as well as dropped
casts when unnecessary.

Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
index c676db69493e41a670da7cab3c66bf9b1042b3c1..077a8500c9ac57b66000ff52955c31b493019d9c 100644 (file)
--- a/grub-core/kern/elf.c
+++ b/grub-core/kern/elf.c
@@ -177,6 +177,7 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #define grub_elfXX_check_endianess_and_bswap_ehdr grub_elf32_check_endianess_and_bswap_ehdr
 #define grub_elfXX_get_shnum grub_elf32_get_shnum
 #define grub_elfXX_get_shstrndx grub_elf32_get_shstrndx
+#define grub_elfXX_get_phnum grub_elf32_get_phnum
 
 #include "elfXX.c"
 
@@ -200,6 +201,7 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #undef grub_elfXX_check_endianess_and_bswap_ehdr
 #undef grub_elfXX_get_shnum
 #undef grub_elfXX_get_shstrndx
+#undef grub_elfXX_get_phnum
 
 \f
 /* 64-bit */
@@ -223,5 +225,6 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #define grub_elfXX_check_endianess_and_bswap_ehdr grub_elf64_check_endianess_and_bswap_ehdr
 #define grub_elfXX_get_shnum grub_elf64_get_shnum
 #define grub_elfXX_get_shstrndx grub_elf64_get_shstrndx
+#define grub_elfXX_get_phnum grub_elf64_get_phnum
 
 #include "elfXX.c"

diff --git a/grub-core/kern/elfXX.c b/grub-core/kern/elfXX.c
index 4e3254fa595a2e5fc91980ea7328c81fa0b98873..aabf4b9d7d41a3e6236437472aee1d1cbb498b1d 100644 (file)
--- a/grub-core/kern/elfXX.c
+++ b/grub-core/kern/elfXX.c
@@ -272,3 +272,37 @@ grub_elfXX_get_shstrndx (ElfXX_Ehdr *e, ElfXX_Word *shstrndx)
     }
   return GRUB_ERR_NONE;
 }
+
+grub_err_t
+grub_elfXX_get_phnum (ElfXX_Ehdr *e, ElfXX_Word *phnum)
+{
+  ElfXX_Shdr *s;
+
+  if (phnum == NULL)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for phnum"));
+
+  /* Set *phnum to 0 so that phnum doesn't return junk on error */
+  *phnum = 0;
+
+  if (e == NULL)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for elf header"));
+
+  *phnum = e->e_phnum;
+  if (*phnum == PN_XNUM)
+    {
+      if (e->e_shoff == 0)
+       return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header table offset in e_shoff"));
+
+      s = (ElfXX_Shdr *) ((grub_uint8_t *) e + e->e_shoff);
+      *phnum = s->sh_info;
+      if (*phnum < PN_XNUM)
+       return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid number of program header table entries in sh_info: %d"), *phnum);
+    }
+  else
+    {
+      if (*phnum >= PN_XNUM)
+       return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid number of program header table entries in e_phnum: %d"), *phnum);
+    }
+
+  return GRUB_ERR_NONE;
+}

diff --git a/grub-core/loader/i386/bsdXX.c b/grub-core/loader/i386/bsdXX.c
index 2d91fd44dd80b925709fe5b05d00267383e03395..dffa79d5692d195cc2137b9bb70ffbad102ce714 100644 (file)
--- a/grub-core/loader/i386/bsdXX.c
+++ b/grub-core/loader/i386/bsdXX.c
@@ -183,6 +183,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator,
   Elf_Ehdr e;
   Elf_Shdr *s, *shdr = NULL;
   Elf_Shnum shnum;
+  Elf_Word phnum;
   grub_addr_t curload, module;
   grub_err_t err;
   grub_size_t chunk_size = 0;
@@ -198,6 +199,10 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator,
   if (err != GRUB_ERR_NONE)
     goto out;
 
+  err = grub_elf_get_phnum (&e, &phnum);
+  if (err != GRUB_ERR_NONE)
+    goto out;
+
   for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum * e.e_shentsize);
        s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
     {
@@ -212,7 +217,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator,
 
   if (chunk_size < sizeof (e))
     chunk_size = sizeof (e);
-  chunk_size += (grub_uint32_t) e.e_phnum * e.e_phentsize;
+  chunk_size += (grub_size_t) phnum * e.e_phentsize;
   chunk_size += (grub_size_t) shnum * e.e_shentsize;
 
   {
@@ -269,9 +274,9 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator,
   curload += (grub_addr_t) shnum * e.e_shentsize;
 
   load (file, argv[0], (grub_uint8_t *) chunk_src + curload - *kern_end, e.e_phoff,
-       (grub_uint32_t) e.e_phnum * e.e_phentsize);
+       (grub_size_t) phnum * e.e_phentsize);
   e.e_phoff = curload - module;
-  curload +=  (grub_uint32_t) e.e_phnum * e.e_phentsize;
+  curload += (grub_addr_t) phnum * e.e_phentsize;
 
   *kern_end = curload;
 

diff --git a/grub-core/loader/multiboot_elfxx.c b/grub-core/loader/multiboot_elfxx.c
index ec4ba3b8d012ff5ce1c1095ac949511035c1c4ec..b6c0e23ca1446c4be7e38bb6248b3ebd4ad8cdee 100644 (file)
--- a/grub-core/loader/multiboot_elfxx.c
+++ b/grub-core/loader/multiboot_elfxx.c
@@ -27,6 +27,7 @@
 # define Elf_Shnum             Elf32_Shnum
 # define grub_multiboot_elf_get_shnum          grub_elf32_get_shnum
 # define grub_multiboot_elf_get_shstrndx       grub_elf32_get_shstrndx
+# define grub_multiboot_elf_get_phnum          grub_elf32_get_phnum
 #elif defined(MULTIBOOT_LOAD_ELF64)
 # define XX                    64
 # define E_MACHINE             MULTIBOOT_ELF64_MACHINE
@@ -38,6 +39,7 @@
 # define Elf_Shnum             Elf64_Shnum
 # define grub_multiboot_elf_get_shnum          grub_elf64_get_shnum
 # define grub_multiboot_elf_get_shstrndx       grub_elf64_get_shstrndx
+# define grub_multiboot_elf_get_phnum          grub_elf64_get_phnum
 #else
 #error "I'm confused"
 #endif
@@ -67,7 +69,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
   grub_relocator_chunk_t ch;
   grub_uint32_t load_offset = 0, load_size;
   Elf_Shnum shnum;
-  Elf_Word shstrndx;
+  Elf_Word shstrndx, phnum;
   unsigned int i;
   void *source = NULL;
 
@@ -93,8 +95,12 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
   if (err != GRUB_ERR_NONE)
     return err;
 
+  err = grub_multiboot_elf_get_phnum (ehdr, &phnum);
+  if (err != GRUB_ERR_NONE)
+    return err;
+
   /* FIXME: Should we support program headers at strange locations?  */
-  if (ehdr->e_phoff + (grub_uint32_t) ehdr->e_phnum * ehdr->e_phentsize > MULTIBOOT_SEARCH)
+  if (ehdr->e_phoff + phnum * ehdr->e_phentsize > MULTIBOOT_SEARCH)
     return grub_error (GRUB_ERR_BAD_OS, "program header at a too high offset");
 
   phdr_base = (char *) mld->buffer + ehdr->e_phoff;
@@ -103,7 +109,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
   mld->link_base_addr = ~0;
 
   /* Calculate lowest and highest load address.  */
-  for (i = 0; i < ehdr->e_phnum; i++)
+  for (i = 0; i < phnum; i++)
     if (phdr(i)->p_type == PT_LOAD)
       {
        mld->link_base_addr = grub_min (mld->link_base_addr, phdr(i)->p_paddr);
@@ -149,7 +155,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
                mld->link_base_addr, mld->load_base_addr);
 
   /* Load every loadable segment in memory.  */
-  for (i = 0; i < ehdr->e_phnum; i++)
+  for (i = 0; i < phnum; i++)
     {
       if (phdr(i)->p_type == PT_LOAD)
         {
@@ -198,7 +204,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
         }
     }
 
-  for (i = 0; i < ehdr->e_phnum; i++)
+  for (i = 0; i < phnum; i++)
     if (phdr(i)->p_vaddr <= ehdr->e_entry
        && phdr(i)->p_vaddr + phdr(i)->p_memsz > ehdr->e_entry)
       {
@@ -220,7 +226,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
        break;
       }
 
-  if (i == ehdr->e_phnum)
+  if (i == phnum)
     return grub_error (GRUB_ERR_BAD_OS, "entry point isn't in a segment");
 
 #if defined (__i386__) || defined (__x86_64__)
@@ -318,3 +324,4 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
 #undef Elf_Shnum
 #undef grub_multiboot_elf_get_shnum
 #undef grub_multiboot_elf_get_shstrndx
+#undef grub_multiboot_elf_get_phnum

diff --git a/include/grub/elf.h b/include/grub/elf.h
index 43c4809ed531306223287a419b4f606d26f2c035..0e70651d4bae199fa061bd346350aed2d7c4ced9 100644 (file)
--- a/include/grub/elf.h
+++ b/include/grub/elf.h
@@ -570,6 +570,7 @@ typedef struct
 #define PT_HIOS                0x6fffffff      /* End of OS-specific */
 #define PT_LOPROC      0x70000000      /* Start of processor-specific */
 #define PT_HIPROC      0x7fffffff      /* End of processor-specific */
+#define PN_XNUM                0xffff
 
 /* Legal values for p_flags (segment flags).  */
 
@@ -2537,9 +2538,11 @@ typedef Elf32_Addr Elf32_Conflict;
 
 extern grub_err_t grub_elf32_get_shnum (Elf32_Ehdr *e, Elf32_Shnum *shnum);
 extern grub_err_t grub_elf32_get_shstrndx (Elf32_Ehdr *e, Elf32_Word *shstrndx);
+extern grub_err_t grub_elf32_get_phnum (Elf32_Ehdr *e, Elf32_Word *phnum);
 
 extern grub_err_t grub_elf64_get_shnum (Elf64_Ehdr *e, Elf64_Shnum *shnum);
 extern grub_err_t grub_elf64_get_shstrndx (Elf64_Ehdr *e, Elf64_Word *shstrndx);
+extern grub_err_t grub_elf64_get_phnum (Elf64_Ehdr *e, Elf64_Word *phnum);
 
 #ifdef GRUB_TARGET_WORDSIZE
 #if GRUB_TARGET_WORDSIZE == 32
@@ -2570,6 +2573,7 @@ typedef Elf32_Shnum Elf_Shnum;
 
 #define grub_elf_get_shnum     grub_elf32_get_shnum
 #define grub_elf_get_shstrndx  grub_elf32_get_shstrndx
+#define grub_elf_get_phnum     grub_elf32_get_phnum
 
 #elif GRUB_TARGET_WORDSIZE == 64
 
@@ -2598,6 +2602,7 @@ typedef Elf64_Shnum Elf_Shnum;
 
 #define grub_elf_get_shnum     grub_elf64_get_shnum
 #define grub_elf_get_shstrndx  grub_elf64_get_shstrndx
+#define grub_elf_get_phnum     grub_elf64_get_phnum
 
 #endif /* GRUB_TARGET_WORDSIZE == 64 */
 #endif