[Qemu-devel] [PATCH 0/5] linux-user: Detect and fix 32-bit guest on 64-bit host issues - v2

[Qemu-devel] [PATCH 0/5] linux-user: Detect and fix 32-bit guest on 64-bit host issues - v2

[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

Jan

[Qemu-devel] [PATCH 1/5] linux-user: Safety belt for h2g

[Jan Kiszka]

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 <jan.kiszka@web.de>
---
 cpu-all.h |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Index: b/cpu-all.h
===================================================================
--- a/cpu-all.h
+++ 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)

[Qemu-devel] [PATCH 2/5] linux-user: Introduce qemu_vmalloc_guest_safe

[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 <jan.kiszka@web.de>
---
 linux-user/mmap.c |   39 +++++++++++++++++++++++++++++++++++++++
 linux-user/qemu.h |    1 +
 2 files changed, 40 insertions(+)

Index: b/linux-user/mmap.c
===================================================================
--- a/linux-user/mmap.c
+++ 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;
Index: b/linux-user/qemu.h
===================================================================
--- a/linux-user/qemu.h
+++ 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 */
 

[Qemu-devel] [PATCH 3/5] linux-user: Allocate guest-reachable descriptor tables

[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 <jan.kiszka@web.de>
---
 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(-)

Index: b/linux-user/main.c
===================================================================
--- a/linux-user/main.c
+++ 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
Index: b/linux-user/i386/syscall.h
===================================================================
--- a/linux-user/i386/syscall.h
+++ 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
 
Index: b/linux-user/x86_64/syscall.h
===================================================================
--- a/linux-user/x86_64/syscall.h
+++ 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. */
Index: b/linux-user/syscall.c
===================================================================
--- a/linux-user/syscall.c
+++ 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);

[Qemu-devel] [PATCH 5/5] linux-user: Fix h2g usage in page_find_alloc

[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 <jan.kiszka@web.de>
---
 exec.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Index: b/exec.c
===================================================================
--- a/exec.c
+++ 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); 

[Qemu-devel] [PATCH 4/5] linux-user: Introduce h2g_valid

[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 <jan.kiszka@web.de>
---
 cpu-all.h |    4 ++++
 1 file changed, 4 insertions(+)

Index: b/cpu-all.h
===================================================================
--- a/cpu-all.h
+++ 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)

Re: [Qemu-devel] [PATCH 0/5] linux-user: Detect and fix 32-bit guest on 64-bit host issues - v2

[Anthony Liguori]

Jan Kiszka wrote:
> 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

> Jan
>
>
>
>
>