ukvm: Audit ELF loader

General cleanup and audit in load_code().  Fixes Solo5#126, removes
unnecessary casts and verifies that the load address/sizes in program
header are valid.

The guard page at the end of the last loaded segment is removed; it
didn't serve any useful function.

ukvm/ukvm-core.c

@@ -154,11 +154,11 @@ ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset)
  *
  * Memory will look like this after the elf is loaded:
  *
- * *mem                    *p_entry                                 *p_end
- *   |             |                    |                |            |
- *   |    ...      | .text .rodata      |   .data .bss   |            |
- *   |             |        code        |   00000000000  | empty page |
- *   |             |  [PROT_EXEC|READ]  |                | PROT_NONE  |
+ * *mem                    *p_entry                   *p_end
+ *   |             |                    |                |
+ *   |    ...      | .text .rodata      |   .data .bss   |
+ *   |             |        code        |   00000000000  |
+ *   |             |  [PROT_EXEC|READ]  |                |
  *
  */
 static void load_code(const char *file, uint8_t *mem,     /* IN */
@@ -186,7 +186,7 @@ static void load_code(const char *file, uint8_t *mem,     /* IN */
     numb = pread_in_full(fd_kernel, &hdr, sizeof(Elf64_Ehdr), 0);
     if (numb < 0)
         err(1, "%s: Could not read ELF header", file);
-    if ((size_t) numb != sizeof(Elf64_Ehdr))
+    if (numb != sizeof(Elf64_Ehdr))
         errx(1, "%s: Not a valid ELF executable", file);
 
     /*
@@ -214,7 +214,7 @@ static void load_code(const char *file, uint8_t *mem,     /* IN */
     numb = pread_in_full(fd_kernel, phdr, buflen, ph_off);
     if (numb < 0)
         err(1, "%s: Read error", file);
-    if ((size_t) numb != buflen)
+    if (numb != buflen)
         err(1, "%s: Invalid program header", file);
 
     /*
@@ -224,45 +224,55 @@ static void load_code(const char *file, uint8_t *mem,     /* IN */
      * ALIGN_UP(p_memsz, p_align) bytes on memory.
      */
     for (ph_i = 0; ph_i < ph_cnt; ph_i++) {
-        uint8_t *dst;
-        size_t _end;
+        uint8_t *daddr;
+        uint64_t _end;
         size_t offset = phdr[ph_i].p_offset;
         size_t filesz = phdr[ph_i].p_filesz;
         size_t memsz = phdr[ph_i].p_memsz;
         uint64_t paddr = phdr[ph_i].p_paddr;
         uint64_t align = phdr[ph_i].p_align;
+        uint64_t result;
 
-        if ((phdr[ph_i].p_type & PT_LOAD) == 0)
+        if (phdr[ph_i].p_type != PT_LOAD)
             continue;
 
         /* XXX this won't work after having the two memory slots */
         assert(GUEST_SIZE < KVM_32BIT_GAP_SIZE);
-        dst = mem + paddr;
-
-        numb = pread_in_full(fd_kernel, dst, filesz, offset);
-        if (numb < 0)
-            err(1, "%s: Read error", file);
-        if ((size_t) numb != filesz)
-            errx(1, "%s: Invalid segment", file);
-
-        memset(mem + paddr + filesz, 0, memsz - filesz);
-
-        /* Protect the executable code */
-        if (phdr[ph_i].p_flags & PF_X)
-            mprotect((void *) dst, memsz, PROT_EXEC | PROT_READ);
 
+        if ((paddr >= GUEST_SIZE) || add_overflow(paddr, filesz, result) ||
+            (result >= GUEST_SIZE)) {
+            errx(1, "%s: Invalid segment: paddr=0x%" PRIx64 ", filesz=%zu",
+                    file, paddr, filesz);
+        }
+        if (add_overflow(paddr, memsz, result) || (result >= GUEST_SIZE)) {
+            errx(1, "%s: Invalid segment: paddr=0x%" PRIx64 ", memsz=%zu",
+                    file, paddr, memsz);
+        }
         _end = ALIGN_UP(paddr + memsz, align);
+        if (_end >= GUEST_SIZE) {
+            errx(1, "%s: Invalid segment: paddr=0x%" PRIx64 \
+                    ", _end=0x%" PRIx64, file, paddr, _end);
+        }
         if (_end > *p_end)
             *p_end = _end;
-    }
 
-    /*
-     * Not needed, but let's give it an empty page at the end for "safety".
-     * And, even protect it against any type of access.
-     */
-    mprotect((void *) ((uint64_t) mem + p_end), 0x1000, PROT_NONE);
-    *p_end += 0x1000;
+        daddr = mem + paddr;
+        numb = pread_in_full(fd_kernel, daddr, filesz, offset);
+        if (numb < 0)
+            err(1, "%s: Read error", file);
+        if (numb != filesz)
+            errx(1, "%s: Short read", file);
+        memset(daddr + filesz, 0, memsz - filesz);
+
+        /* Write-protect the executable segment */
+        if (phdr[ph_i].p_flags & PF_X) {
+            if (mprotect(daddr, _end - paddr, PROT_EXEC | PROT_READ) == -1)
+                errx(1, "%s: mprotect() failed, daddr=%p, paddr=0x%lx",
+                        file, daddr, paddr);
+        }
+    }
 
+    close (fd_kernel);
     *p_entry = hdr.e_entry;
 }