Skip to contentSkip to conversation

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

PATCH In review Jan Kiszka 2 participants 3 messages Jul 13, 2008 – Jul 23, 2008

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

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

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.

This patch, together with the one for gdt_table, fixes the reported qemu-i386 regression [1].

[1] http://permalink.gmane.org/gmane.comp.emulators.qemu/26987

Signed-off-by Jan Kiszka <[email protected]>
---
 exec.c            |   17 ++++++++++-------
 linux-user/mmap.c |    2 +-
 linux-user/qemu.h |    1 +
 3 files changed, 12 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
===================================================================
@@ -260,7 +260,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 +11 -7
Index: b/exec.c
===================================================================
@@ -295,17 +295,20 @@ static inline PageDesc *page_find_alloc(
         /* allocate if not found */
 #if defined(CONFIG_USER_ONLY)
         unsigned long addr;
+        abi_ulong mmap_start;
         size_t len = sizeof(PageDesc) * L2_SIZE;
-        /* Don't use qemu_malloc because it may recurse.  */
-        p = mmap(0, len, PROT_READ | PROT_WRITE,
+        abi_ulong host_len = HOST_PAGE_ALIGN(len);
+
+        /* Ensure we allocate from the guest-reachable rage */
+        mmap_start = mmap_find_vma(0, host_len);
+        assert(mmap_start != (abi_ulong)-1);
+        p = mmap(g2h(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;
#1 lore raw
Quoted 3 lines · 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.

I'm pretty sure this is wrong. The structures allocated by page_find_alloc do not need to be accessible by the guest. In fact it's better if they are outside the guest address space.

Paul

author
#2 lore raw
Quoted 8 lines · Paul Brook
Quoted 3 lines · 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.

I'm pretty sure this is wrong. The structures allocated by page_find_alloc do not need to be accessible by the guest. In fact it's better if they are outside the guest address space.

Bet we have "addr = h2g(p);" in page_find_alloc(), ie. we expect to retrieve a valid guest address (for use with page_set_flag). Something is borken here, but you can surely better tell us what precisely than I'm able to.

Jan