[Qemu-devel] [PATCH] linux-user: Fix page_find_alloc for 32-bit use on 64-bit hosts

[Qemu-devel] [PATCH] linux-user: Fix page_find_alloc for 32-bit use on 64-bit hosts

[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.

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 <jan.kiszka@web.de>
---
 exec.c            |   17 ++++++++++-------
 linux-user/mmap.c |    2 +-
 linux-user/qemu.h |    1 +
 3 files changed, 12 insertions(+), 8 deletions(-)

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

Re: [Qemu-devel] [PATCH] linux-user: Fix page_find_alloc for 32-bit use on 64-bit hosts

[Paul Brook]

On Sunday 13 July 2008, Jan Kiszka wrote:
> 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

Re: [Qemu-devel] [PATCH] linux-user: Fix page_find_alloc for 32-bit use on 64-bit hosts

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 696 bytes --]

Paul Brook wrote:
> On Sunday 13 July 2008, Jan Kiszka wrote:
>> 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]