Skip to contentSkip to conversation

linux-user: Detect and fix 32-bit guest on 64-bit host issues

PATCH New Jan Kiszka 1 participant 5 messages Jul 14, 2008

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

cover linux-user: Detect and fix 32-bit guest on 64-bit host issues

This series deals with address space mapping issues around h2g/g2h that currently exist on 64-bit hosts running 32-bit Linux user-space guests.

The first patch is an unmodified repost of the h2g instrumentation to detect such issues, the other three patches are a reworked approach to fix the current issues (specifically on x86).

This series fixes the problem reported here:

    http://permalink.gmane.org/gmane.comp.emulators.qemu/26987

Jan

author
#1 lore raw

PATCH 1/4 linux-user: Safety belt for h2g

h2g can only work on 64-bit hosts if the provided address is mappable to the guest range. Neglecting this was already the source for several bugs. Instrument the macro so that it will trigger earlier in the future (at least as long as we have this kind of mapping mechanism).

---
 cpu-all.h |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
cpu-all.h +7 -1
Index: b/cpu-all.h
===================================================================
@@ -659,6 +659,8 @@ static inline void stfq_be_p(void *ptr,
 /* MMU memory access macros */
 
 #if defined(CONFIG_USER_ONLY)
+#include <assert.h>
+
 /* On some host systems the guest address space is reserved on the host.
  * This allows the guest address space to be offset to a convenient location.
  */
@@ -667,7 +669,11 @@ static inline void stfq_be_p(void *ptr,
 
 /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
 #define g2h(x) ((void *)((unsigned long)(x) + GUEST_BASE))
-#define h2g(x) ((target_ulong)((unsigned long)(x) - GUEST_BASE))
+#define h2g(x) ({ \
+    unsigned long __ret = (unsigned long)(x) - GUEST_BASE; \
+    assert(__ret == (target_ulong)__ret); \
+    __ret; \
+})
 
 #define saddr(x) g2h(x)
 #define laddr(x) g2h(x)
author
#2 lore raw

PATCH 2/4 linux-user: Introduce qemu_vmalloc_guest_safe

In reply to Jan Kiszka

Add qemu_vmalloc_guest_safe to allocate memory from a region that are reachable by user-space guests. Addresses returned by this function can safely be provided to h2g.

---
 linux-user/mmap.c |   37 +++++++++++++++++++++++++++++++++++++
 linux-user/qemu.h |    1 +
 2 files changed, 38 insertions(+)
linux-user/mmap.c +37 -0
Index: b/linux-user/mmap.c
===================================================================
@@ -94,6 +94,43 @@ void *qemu_vmalloc(size_t size)
     return p;
 }
 
+void *qemu_vmalloc_guest_safe(size_t size)
+{
+#if TARGET_LONG_BIT < HOST_LONG_BITS
+    void *p;
+    unsigned long addr;
+    abi_ulong guest_start;
+
+    mmap_lock();
+
+    /* Ensure we allocate from the guest-reachable rage */
+    size = HOST_PAGE_ALIGN(size);
+    guest_start = mmap_find_vma(0, size);
+    if (guest_start == (abi_ulong)-1) {
+        p = NULL;
+        goto unlock_exit;
+    }
+
+    /* Use map and mark the pages as used.  */
+    p = mmap(g2h(guest_start), size, PROT_READ | PROT_WRITE,
+             MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+    if (p == MAP_FAILED) {
+        p = NULL;
+        goto unlock_exit;
+    }
+
+    addr = (unsigned long)p;
+    page_set_flags(addr & TARGET_PAGE_MASK, TARGET_PAGE_ALIGN(addr + size),
+                   PAGE_RESERVED);
+
+unlock_exit:
+    mmap_unlock();
+    return p;
+#else
+    return qemu_vmalloc(size);
+#endif
+}
+
 void *qemu_malloc(size_t size)
 {
     char * p;
linux-user/qemu.h +1 -0
Index: b/linux-user/qemu.h
===================================================================
@@ -247,6 +247,7 @@ void mmap_unlock(void);
 void mmap_fork_start(void);
 void mmap_fork_end(int child);
 #endif
+void *qemu_vmalloc_guest_safe(size_t size);
 
 /* user access */
 
author
#3 lore raw

PATCH 3/4 linux-user: Allocate guest-reachable descriptor tables

In reply to Jan Kiszka

IDT, GDT and LDTs have to allocated from memory regions also reachable by the guests. Obtain them via the new qemu_vmalloc_guest_safe service.

---
 linux-user/i386/syscall.h   |    2 ++
 linux-user/main.c           |   13 +++++++------
 linux-user/syscall.c        |    3 ++-
 linux-user/x86_64/syscall.h |    2 ++
 4 files changed, 13 insertions(+), 7 deletions(-)
linux-user/main.c +7 -6
Index: b/linux-user/main.c
===================================================================
@@ -280,9 +280,9 @@ static void write_dt(void *ptr, unsigned
     p[1] = tswapl(e2);
 }
 
-#if TARGET_X86_64
-uint64_t idt_table[512];
+uint64_t *idt_table;
 
+#if TARGET_X86_64
 static void set_gate64(void *ptr, unsigned int type, unsigned int dpl,
                        uint64_t addr, unsigned int sel)
 {
@@ -301,8 +301,6 @@ static void set_idt(int n, unsigned int
     set_gate64(idt_table + n * 2, 0, dpl, 0, 0);
 }
 #else
-uint64_t idt_table[256];
-
 static void set_gate(void *ptr, unsigned int type, unsigned int dpl,
                      uint32_t addr, unsigned int sel)
 {
@@ -2444,8 +2442,10 @@ int main(int argc, char **argv)
 #endif
 
     /* linux interrupt setup */
+    idt_table = qemu_vmalloc_guest_safe(sizeof(uint64_t) * TARGET_IDT_ENTRIES);
+    memset(idt_table, 0, sizeof(uint64_t) * TARGET_IDT_ENTRIES);
     env->idt.base = h2g(idt_table);
-    env->idt.limit = sizeof(idt_table) - 1;
+    env->idt.limit = sizeof(uint64_t) * TARGET_IDT_ENTRIES - 1;
     set_idt(0, 0);
     set_idt(1, 0);
     set_idt(2, 0);
@@ -2471,7 +2471,8 @@ int main(int argc, char **argv)
     /* linux segment setup */
     {
         uint64_t *gdt_table;
-        gdt_table = qemu_mallocz(sizeof(uint64_t) * TARGET_GDT_ENTRIES);
+        gdt_table = qemu_vmalloc_guest_safe(sizeof(uint64_t) * TARGET_GDT_ENTRIES);
+        memset(gdt_table, 0, sizeof(uint64_t) * TARGET_GDT_ENTRIES);
         env->gdt.base = h2g((unsigned long)gdt_table);
         env->gdt.limit = sizeof(uint64_t) * TARGET_GDT_ENTRIES - 1;
 #ifdef TARGET_ABI32
linux-user/i386/syscall.h +2 -0
Index: b/linux-user/i386/syscall.h
===================================================================
@@ -22,6 +22,8 @@ struct target_pt_regs {
 
 /* ioctls */
 
+#define TARGET_IDT_ENTRIES      256
+
 #define TARGET_LDT_ENTRIES      8192
 #define TARGET_LDT_ENTRY_SIZE	8
 
linux-user/x86_64/syscall.h +2 -0
Index: b/linux-user/x86_64/syscall.h
===================================================================
@@ -29,6 +29,8 @@ struct target_pt_regs {
 /* top of stack page */
 };
 
+#define TARGET_IDT_ENTRIES      512
+
 /* Maximum number of LDT entries supported. */
 #define TARGET_LDT_ENTRIES	8192
 /* The size of each LDT entry. */
linux-user/syscall.c +2 -1
Index: b/linux-user/syscall.c
===================================================================
@@ -2473,7 +2473,8 @@ static abi_long write_ldt(CPUX86State *e
     }
     /* allocate the LDT */
     if (!ldt_table) {
-        ldt_table = malloc(TARGET_LDT_ENTRIES * TARGET_LDT_ENTRY_SIZE);
+        ldt_table = qemu_vmalloc_guest_safe(TARGET_LDT_ENTRIES
+                                            * TARGET_LDT_ENTRY_SIZE);
         if (!ldt_table)
             return -TARGET_ENOMEM;
         memset(ldt_table, 0, TARGET_LDT_ENTRIES * TARGET_LDT_ENTRY_SIZE);
author
#4 lore raw

PATCH 4/4 linux-user: Fix page_find_alloc for 32-bit use on 64-bit hosts

In reply to Jan Kiszka

page_find_alloc, used e.g. for TB allocation, is not safe on 64-bit hosts for 32-bit guests. Patch below fixes this by requesting new pages only from the guest-reachable address range.

---
 exec.c            |   24 +++++++++++++++++-------
 linux-user/mmap.c |    2 +-
 linux-user/qemu.h |    1 +
 3 files changed, 19 insertions(+), 8 deletions(-)
linux-user/qemu.h +1 -0
Index: b/linux-user/qemu.h
===================================================================
@@ -232,6 +232,7 @@ void sparc64_get_context(CPUSPARCState *
 #endif
 
 /* mmap.c */
+abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size);
 int target_mprotect(abi_ulong start, abi_ulong len, int prot);
 abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
                      int flags, int fd, abi_ulong offset);
linux-user/mmap.c +1 -1
Index: b/linux-user/mmap.c
===================================================================
@@ -297,7 +297,7 @@ unsigned long last_brk;
 */
 /* page_init() marks pages used by the host as reserved to be sure not
    to use them. */
-static abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size)
+abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size)
 {
     abi_ulong addr, addr1, addr_start;
     int prot;
exec.c +17 -7
Index: b/exec.c
===================================================================
@@ -294,18 +294,28 @@ static inline PageDesc *page_find_alloc(
     if (!p) {
         /* allocate if not found */
 #if defined(CONFIG_USER_ONLY)
+        void *start = NULL;
         unsigned long addr;
         size_t len = sizeof(PageDesc) * L2_SIZE;
-        /* Don't use qemu_malloc because it may recurse.  */
-        p = mmap(0, len, PROT_READ | PROT_WRITE,
+
+#if TARGET_LONG_BITS < HOST_LONG_BITS
+        {
+            /* Ensure we allocate from the guest-reachable rage */
+            abi_ulong guest_start;
+
+            len = HOST_PAGE_ALIGN(len);
+            guest_start = mmap_find_vma(0, len);
+            assert(guest_start != (abi_ulong)-1);
+            start = g2h(guest_start);
+        }
+#endif
+        p = mmap(start, len, PROT_READ | PROT_WRITE,
                  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+        assert(p != MAP_FAILED);
         *lp = p;
         addr = h2g(p);
-        if (addr == (target_ulong)addr) {
-            page_set_flags(addr & TARGET_PAGE_MASK,
-                           TARGET_PAGE_ALIGN(addr + len),
-                           PAGE_RESERVED); 
-        }
+        page_set_flags(addr & TARGET_PAGE_MASK, TARGET_PAGE_ALIGN(addr + len),
+                       PAGE_RESERVED);
 #else
         p = qemu_mallocz(sizeof(PageDesc) * L2_SIZE);
         *lp = p;