[PATCH v3 1/2] ntdll: Try mapping free memory region outside of reserved regions.

Rémi Bernon rbernon at codeweavers.com
Thu Nov 21 06:35:25 CST 2019


We assumed that all mapped regions are known by Wine view tree, which
is obviously not the case with external allocations. This could lead to
memory corruption when find_free_area returns an expected free region
which is already mapped. Using MAP_FIXED forces mmap to succeed and
corrupts the mapping.

This patch uses safe mmap calls to check if the expected free memory is
actually free, and iterates over the expected free ranges until it
succeeds.

Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
---
 dlls/ntdll/virtual.c | 64 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c
index 68249de902e..60838c68a55 100644
--- a/dlls/ntdll/virtual.c
+++ b/dlls/ntdll/virtual.c
@@ -496,6 +496,35 @@ static struct file_view *find_view_range( const void *addr, size_t size )
     return NULL;
 }
 
+/***********************************************************************
+ *           try_map_free_area
+ *
+ * Try mmaping some expected free memory region, eventually stepping and
+ * retrying inside it, and return where it actually succeeded, or NULL.
+ */
+static void* try_map_free_area( void *base, void *end, ptrdiff_t step,
+                                void *start, size_t size, int unix_prot )
+{
+    void *ptr;
+
+    while (start && base <= start && (char*)start + size <= (char*)end)
+    {
+        if ((ptr = wine_anon_mmap( start, size, unix_prot, 0 )) == start)
+            return start;
+        TRACE( "Found free area is already mapped, start %p.\n", start );
+
+        if (ptr != (void *)-1)
+            munmap( ptr, size );
+
+        if ((step > 0 && (char *)end - (char *)start < step) ||
+            (step < 0 && (char *)start - (char *)base < -step) ||
+            step == 0)
+            break;
+        start = (char *)start + step;
+    }
+
+    return NULL;
+}
 
 /***********************************************************************
  *           find_free_area
@@ -503,9 +532,11 @@ static struct file_view *find_view_range( const void *addr, size_t size )
  * Find a free area between views inside the specified range.
  * The csVirtual section must be held by caller.
  */
-static void *find_free_area( void *base, void *end, size_t size, size_t mask, int top_down )
+static void *find_free_area( void *base, void *end, size_t size, size_t mask, int top_down,
+                             int map_area, int unix_prot )
 {
     struct wine_rb_entry *first = NULL, *ptr = views_tree.root;
+    ptrdiff_t step = top_down ? -(mask + 1) : (mask + 1);
     void *start;
 
     /* find the first (resp. last) view inside the range */
@@ -538,7 +569,10 @@ static void *find_free_area( void *base, void *end, size_t size, size_t mask, in
         {
             struct file_view *view = WINE_RB_ENTRY_VALUE( first, struct file_view, entry );
 
-            if ((char *)view->base + view->size <= (char *)start) break;
+            if (!map_area && (char *)view->base + view->size <= (char *)start) break;
+            if (map_area && (start = try_map_free_area( (char *)view->base + view->size,
+                                                        (char *)start + size, step,
+                                                        start, size, unix_prot ))) break;
             start = ROUND_ADDR( (char *)view->base - size, mask );
             /* stop if remaining space is not large enough */
             if (!start || start >= end || start < base) return NULL;
@@ -554,13 +588,17 @@ static void *find_free_area( void *base, void *end, size_t size, size_t mask, in
         {
             struct file_view *view = WINE_RB_ENTRY_VALUE( first, struct file_view, entry );
 
-            if ((char *)view->base >= (char *)start + size) break;
+            if (!map_area && (char *)view->base >= (char *)start + size) break;
+            if (map_area && (start = try_map_free_area( start, view->base, step,
+                                                        start, size, unix_prot ))) break;
             start = ROUND_ADDR( (char *)view->base + view->size + mask, mask );
             /* stop if remaining space is not large enough */
             if (!start || start >= end || (char *)end - (char *)start < size) return NULL;
             first = wine_rb_next( first );
         }
     }
+
+    if (!first && map_area) return try_map_free_area( base, end, step, start, size, unix_prot );
     return start;
 }
 
@@ -1054,13 +1092,13 @@ static int alloc_reserved_area_callback( void *start, size_t size, void *arg )
         {
             /* range is split in two by the preloader reservation, try first part */
             if ((alloc->result = find_free_area( start, preload_reserve_start, alloc->size,
-                                                 alloc->mask, alloc->top_down )))
+                                                 alloc->mask, alloc->top_down, FALSE, 0 )))
                 return 1;
             /* then fall through to try second part */
             start = preload_reserve_end;
         }
     }
-    if ((alloc->result = find_free_area( start, end, alloc->size, alloc->mask, alloc->top_down )))
+    if ((alloc->result = find_free_area( start, end, alloc->size, alloc->mask, alloc->top_down, FALSE, 0 )))
         return 1;
 
     return 0;
@@ -1148,6 +1186,7 @@ static NTSTATUS map_view( struct file_view **view_ret, void *base, size_t size,
     {
         size_t view_size = size + mask + 1;
         struct alloc_area alloc;
+        int unix_prot = VIRTUAL_GetUnixProt(vprot);
 
         alloc.size = size;
         alloc.mask = mask;
@@ -1158,19 +1197,22 @@ static NTSTATUS map_view( struct file_view **view_ret, void *base, size_t size,
         {
             ptr = alloc.result;
             TRACE( "got mem in reserved area %p-%p\n", ptr, (char *)ptr + size );
-            if (wine_anon_mmap( ptr, size, VIRTUAL_GetUnixProt(vprot), MAP_FIXED ) != ptr)
+            if (wine_anon_mmap( ptr, size, unix_prot, MAP_FIXED ) != ptr)
                 return STATUS_INVALID_PARAMETER;
             goto done;
         }
 
-        for (;;)
+        if (zero_bits_64)
         {
-            if (!zero_bits_64)
-                ptr = NULL;
-            else if (!(ptr = find_free_area( (void*)0, alloc.limit, view_size, mask, top_down )))
+            if (!(ptr = find_free_area( (void*)0, alloc.limit, size, mask, top_down, TRUE, unix_prot )))
                 return STATUS_NO_MEMORY;
+            TRACE( "got mem with find_free_area %p-%p\n", ptr, (char *)ptr + size );
+            goto done;
+        }
 
-            if ((ptr = wine_anon_mmap( ptr, view_size, VIRTUAL_GetUnixProt(vprot), ptr ? MAP_FIXED : 0 )) == (void *)-1)
+        for (;;)
+        {
+            if ((ptr = wine_anon_mmap( NULL, view_size, unix_prot, 0 )) == (void *)-1)
             {
                 if (errno == ENOMEM) return STATUS_NO_MEMORY;
                 return STATUS_INVALID_PARAMETER;
-- 
2.24.0.rc2




More information about the wine-devel mailing list