libelf: fix symtab/strtab loading for 32bit domains

Commit ed04ca introduced a bug in the symtab/strtab loading for 32bit
guests, that corrupted the section headers array due to the padding
introduced by the elf_shdr union.

The Elf section header array on 32bit should be accessible as an array of
Elf32_Shdr elements, and the union with Elf64_Shdr done in elf_shdr was
breaking this due to size differences between Elf32_Shdr and Elf64_Shdr.

Fix this by copying each section header one by one, and using the proper
size depending on the bitness of the guest kernel. While there, also fix
a couple of consistency issues, by making sure we always use the sizes of
our local versions of the ELF header and the ELF sections headers.

Reported-by: Brian Marcotte <marcotte@panix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>

diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c
index d67e0a71db5bce8d1950bca45de5e41e21c468ce..4e12a715b6219c5f7bb7d7ca5d0af5526f9e2bca 100644 (file)
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -21,10 +21,17 @@
 
 #include "libelf-private.h"
 
+/* ------------------------------------------------------------------------ */
+
 /* Number of section header needed in order to fit the SYMTAB and STRTAB. */
 #define ELF_BSDSYM_SECTIONS 3
-
-/* ------------------------------------------------------------------------ */
+struct elf_sym_header {
+    uint32_t size;
+    struct {
+        elf_ehdr header;
+        elf_shdr section[ELF_BSDSYM_SECTIONS];
+    } elf_header;
+} __attribute__((packed));
 
 elf_errorstatus elf_init(struct elf_binary *elf, const char *image_input, size_t size)
 {
@@ -172,9 +179,10 @@ void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t pstart)
     /* Space to store the size of the elf image */
     sz = sizeof(uint32_t);
 
-    /* Space for the elf and elf section headers */
-    sz += elf_uval(elf, elf->ehdr, e_ehsize) +
-          ELF_BSDSYM_SECTIONS * elf_uval(elf, elf->ehdr, e_shentsize);
+    /* Space for the ELF header and section headers */
+    sz += offsetof(struct elf_sym_header, elf_header.section) +
+          ELF_BSDSYM_SECTIONS * (elf_64bit(elf) ? sizeof(Elf64_Shdr) :
+                                                  sizeof(Elf32_Shdr));
     sz = elf_round_up(elf, sz);
 
     /*
@@ -251,18 +259,11 @@ static void elf_load_bsdsyms(struct elf_binary *elf)
      * strtab, so we only need three section headers in our fake ELF
      * header (first section header is always the undefined section).
      */
-    struct {
-        uint32_t size;
-        struct {
-            elf_ehdr header;
-            elf_shdr section[ELF_BSDSYM_SECTIONS];
-        } __attribute__((packed)) elf_header;
-    } __attribute__((packed)) header;
-
+    struct elf_sym_header header;
     ELF_HANDLE_DECL(elf_ehdr) header_handle;
-    unsigned long shdr_size;
+    unsigned long shdr_size, ehdr_size, header_size;
     ELF_HANDLE_DECL(elf_shdr) section_handle;
-    unsigned int link, rc;
+    unsigned int link, rc, i;
     elf_ptrval header_base;
     elf_ptrval elf_header_base;
     elf_ptrval symtab_base;
@@ -297,12 +298,28 @@ do {                                                                \
                       sizeof(uint32_t);
     symtab_base = elf_round_up(elf, header_base + sizeof(header));
 
+    /*
+     * Set the size of the ELF header and the section headers, based on the
+     * size of our local copy.
+     */
+    ehdr_size = elf_64bit(elf) ? sizeof(header.elf_header.header.e64) :
+                                 sizeof(header.elf_header.header.e32);
+    shdr_size = elf_64bit(elf) ? sizeof(header.elf_header.section[0].e64) :
+                                 sizeof(header.elf_header.section[0].e32);
+
     /* Fill the ELF header, copied from the original ELF header. */
     header_handle = ELF_MAKE_HANDLE(elf_ehdr,
                                 ELF_REALPTR2PTRVAL(&header.elf_header.header));
     elf_memcpy_safe(elf, ELF_HANDLE_PTRVAL(header_handle),
-                    ELF_HANDLE_PTRVAL(elf->ehdr),
-                    elf_uval(elf, elf->ehdr, e_ehsize));
+                    ELF_HANDLE_PTRVAL(elf->ehdr), ehdr_size);
+
+    /*
+     * Set the ELF header size, section header entry size and version
+     * (in case we are dealing with an input ELF header that has extensions).
+     */
+    elf_store_field_bitness(elf, header_handle, e_ehsize, ehdr_size);
+    elf_store_field_bitness(elf, header_handle, e_shentsize, shdr_size);
+    elf_store_field_bitness(elf, header_handle, e_version, EV_CURRENT);
 
     /* Set the offset to the shdr array. */
     elf_store_field_bitness(elf, header_handle, e_shoff,
@@ -315,8 +332,7 @@ do {                                                                \
     elf_store_field_bitness(elf, header_handle, e_phoff, 0);
     elf_store_field_bitness(elf, header_handle, e_phentsize, 0);
     elf_store_field_bitness(elf, header_handle, e_phnum, 0);
-
-    shdr_size = elf_uval(elf, elf->ehdr, e_shentsize);
+    elf_store_field_bitness(elf, header_handle, e_shstrndx, 0);
 
     /*
      * The symtab section header is going to reside in section[SYMTAB_INDEX],
@@ -387,15 +403,35 @@ do {                                                                \
     header.size = strtab_base + elf_uval(elf, section_handle, sh_size) -
                   elf_header_base;
 
-    /* Load the headers. */
+    /* Load the size plus ELF header. */
+    header_size = offsetof(typeof(header), elf_header.section);
     rc = elf_load_image(elf, header_base, ELF_REALPTR2PTRVAL(&header),
-                        sizeof(header), sizeof(header));
+                        header_size, header_size);
     if ( rc != 0 )
     {
         elf_mark_broken(elf, "unable to load ELF headers into guest memory");
         return;
     }
 
+    /*
+     * Load the section headers.
+     *
+     * NB: this _must_ be done one by one, and taking the bitness into account,
+     * so that the guest can treat this as an array of type Elf{32/64}_Shdr.
+     */
+    for ( i = 0; i < ELF_BSDSYM_SECTIONS; i++ )
+    {
+        rc = elf_load_image(elf, header_base + header_size + shdr_size * i,
+                            ELF_REALPTR2PTRVAL(&header.elf_header.section[i]),
+                            shdr_size, shdr_size);
+        if ( rc != 0 )
+        {
+            elf_mark_broken(elf,
+                        "unable to load ELF section header into guest memory");
+            return;
+        }
+    }
+
     /* Remove permissions from elf_memcpy_safe. */
     elf_set_xdest(elf, NULL, 0);