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

PATCH In review Jan Kiszka 2 participants 7 messages Aug 16, 2008 – Aug 21, 2008

Share & tools

Jan Kiszka

author

2008-08-16 09:39 UTC #1 lore raw

PATCH 1/5 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).

Signed-off-by Jan Kiszka <[email protected]>

---
 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)

Jan Kiszka

author

2008-08-16 09:39 UTC #2 lore raw

PATCH 2/5 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.

Signed-off-by Jan Kiszka <[email protected]>

---
 linux-user/mmap.c |   39 +++++++++++++++++++++++++++++++++++++++
 linux-user/qemu.h |    1 +
 2 files changed, 40 insertions(+)

linux-user/mmap.c +39 -0

Index: b/linux-user/mmap.c
===================================================================
@@ -94,6 +94,45 @@ void *qemu_vmalloc(size_t size)
     return p;
 }
 
+static abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size);
+
+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 */

Jan Kiszka

author

2008-08-16 09:39 UTC #3 lore raw

PATCH 3/5 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.

Signed-off-by Jan Kiszka <[email protected]>

---
 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
===================================================================
@@ -2474,7 +2474,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);

Jan Kiszka

author

2008-08-16 09:39 UTC #4 lore raw

PATCH 4/5 linux-user: Introduce h2g_valid

In reply to Jan Kiszka

Introduce h2g_valid to check if a given host address can be converted into a valid guest address.

Signed-off-by Jan Kiszka <[email protected]>

---
 cpu-all.h |    4 ++++
 1 file changed, 4 insertions(+)

cpu-all.h +4 -0

Index: b/cpu-all.h
===================================================================
@@ -674,6 +674,10 @@ static inline void stfq_be_p(void *ptr,
     assert(__ret == (target_ulong)__ret); \
     __ret; \
 })
+#define h2g_valid(x) ({ \
+    unsigned long __guest = (unsigned long)(x) - GUEST_BASE; \
+    (__guest == (target_ulong)__guest); \
+})
 
 #define saddr(x) g2h(x)
 #define laddr(x) g2h(x)

Jan Kiszka

author

2008-08-16 09:42 UTC #5 lore raw

PATCH 5/5 linux-user: Fix h2g usage in page_find_alloc

In reply to Jan Kiszka

Paul's comment on my first approach to fix the h2g usage in page_find_alloc finally open my eyes about what the code is actually supposed to do:

With the help of h2g_valid we can no cleanly check if a freshly allocate page (for host usage) is guest-reachable and, in case it is, mark it reserved in the guest's address range.

Signed-off-by Jan Kiszka <[email protected]>

---
 exec.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

exec.c +2 -3

Index: b/exec.c
===================================================================
@@ -294,14 +294,13 @@ static inline PageDesc *page_find_alloc(
     if (!p) {
         /* allocate if not found */
 #if defined(CONFIG_USER_ONLY)
-        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,
                  MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
         *lp = p;
-        addr = h2g(p);
-        if (addr == (target_ulong)addr) {
+        if (h2g_valid(p)) {
+            unsigned long addr = h2g(p);
             page_set_flags(addr & TARGET_PAGE_MASK,
                            TARGET_PAGE_ALIGN(addr + len),
                            PAGE_RESERVED);

Anthony Liguori

2008-08-21 19:42 UTC #6 lore raw

In reply to Jan Kiszka

Quoted 11 lines · Jan Kiszka

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 3 patches are taken unmodified from -v1 of this series (except for a compiler warning fix in 2). The 4th and 5th are new, hopefully addressing the bug in page_find_alloc correctly now.
This series fixes the problem reported here:
    http://permalink.gmane.org/gmane.comp.emulators.qemu/26987

linux-user currently lacks a maintainer so I don't know who the right person is to review these patches. I'm not at all familiar with the code myself.

Regards,

Anthony Liguori

Quoted 6 lines

Jan