[Qemu-devel] [PATCH] mixed types in target_mremap() return incorrect value

[Qemu-devel] [PATCH] mixed types in target_mremap() return incorrect value

[Thayne Harbaugh]

There are mixed types (long vs target_ulong) in
linux-user/mmap.c:target_mremap() and consequently the wrong value is
returned (automatic type casting fails) when the host long is larger
than the guest long and -1 is returned for an error condition.

Consider the initial lines of target_mremap() where a failure is tested:

long target_mremap(target_ulong old_addr, target_ulong old_size,
                   target_ulong new_size, unsigned long flags,
                   target_ulong new_addr)
{
    int prot;

    /* XXX: use 5 args syscall */
    new_addr = (long)mremap(g2h(old_addr), old_size, new_size, flags);
    if (new_addr == -1)
        return new_addr;

Note that signed long is the return type for target_mremap() while
target_ulong is used for new_addr and the return value of mremap().
When the host long is larger than the target long the following can
happen on mremap() failure: -1 is stored in new_addr which is unsigned.
The test for new_addr == -1 is true because the -1 is cast to
target_ulong.  When new_addr is returned, however, new_addr is cast to
long, which is larger than target_ulong for the case of i386 target on
x86_64 guest and the -1 is now a positive number that can be fully
represented as a positive number even though the return value is signed.

This patch is one way to ensure that -1 is always returned for an error
condition:

Index: qemu/linux-user/mmap.c
===================================================================
--- qemu.orig/linux-user/mmap.c	2007-10-02 00:27:36.000000000 -0600
+++ qemu/linux-user/mmap.c	2007-10-02 00:31:41.000000000 -0600
@@ -395,7 +395,7 @@
     /* XXX: use 5 args syscall */
     new_addr = (long)mremap(g2h(old_addr), old_size, new_size, flags);
     if (new_addr == -1)
-        return new_addr;
+        return -1;
     new_addr = h2g(new_addr);
     prot = page_get_flags(old_addr);
     page_set_flags(old_addr, old_addr + old_size, 0);