[Qemu-devel] [PATCH] Fix brk syscall error handling and mmap issues

[Qemu-devel] [PATCH] Fix brk syscall error handling and mmap issues

[Richard Purdie]

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

When running certain memory heavy programs under QEMU ARM user emulation
I've noticed a QEMU segfault.

The segfault is occurring just after an mmap call on the host system.
The reason is that the target application tries to enlarge the data
segment over the top of the memory address where the qemu binary is
loaded using a brk syscall. The brk syscall is translated into an mmap
call which unmaps the qemu binary from memory after which it
unsurprisingly segfaults.

Initially I tested this by changing the load address of the QEMU binary
but that fix is less than ideal since its still possible to conflict.
I've attached a better which where the mmap code checks the call doesn't
touch "reserved" pages and returns an error if it does instead of
calling mmap. 

brk syscalls usually occur from within malloc within glibc. If they
fail, malloc should start using mmap to request memory instead. The
above error should therefore not be fatal and the application should
just stop making brk calls and use mmap but this isn't what was
observed, it continued to segfault somewhere else in the program.

I found that the error return values for the brk syscall were incorrect
and were causing this secondary segfault. In case of an error, the brk
syscall should return the original brk value, not any kind of error
code.

The attached patch fixes both these issues.

Cheers,

Richard

[-- Attachment #2: fix_brk.patch --]
[-- Type: text/x-patch, Size: 1804 bytes --]

Index: linux-user/syscall.c
===================================================================
--- linux-user/syscall.c	(revision 16)
+++ linux-user/syscall.c	(working copy)
@@ -441,7 +441,7 @@
     if (!new_brk)
         return target_brk;
     if (new_brk < target_original_brk)
-        return -TARGET_ENOMEM;
+        return target_brk;
 
     brk_page = HOST_PAGE_ALIGN(target_brk);
 
@@ -456,12 +456,11 @@
     mapped_addr = get_errno(target_mmap(brk_page, new_alloc_size,
                                         PROT_READ|PROT_WRITE,
                                         MAP_ANON|MAP_FIXED|MAP_PRIVATE, 0, 0));
-    if (is_error(mapped_addr)) {
-	return mapped_addr;
-    } else {
+
+    if (!is_error(mapped_addr))
 	target_brk = new_brk;
-    	return target_brk;
-    }
+    
+    return target_brk;
 }
 
 static inline abi_long copy_from_user_fdset(fd_set *fds,
Index: linux-user/mmap.c
===================================================================
--- linux-user/mmap.c	(revision 16)
+++ linux-user/mmap.c	(working copy)
@@ -260,6 +259,9 @@
             host_start += offset - host_offset;
         start = h2g(host_start);
     } else {
+        int flg;
+        target_ulong addr;
+
         if (start & ~TARGET_PAGE_MASK) {
             errno = EINVAL;
             return -1;
@@ -267,6 +269,14 @@
         end = start + len;
         real_end = HOST_PAGE_ALIGN(end);
         
+        for(addr = real_start; addr < real_end; addr += TARGET_PAGE_SIZE) {
+            flg = page_get_flags(addr);
+            if( flg & PAGE_RESERVED ) {
+                errno = ENXIO;
+                return -1;
+            }
+        }
+
         /* worst case: we cannot map the file because the offset is not
            aligned, so we read it */
         if (!(flags & MAP_ANONYMOUS) &&