Modify the NaCl_page_alloc_hint function to use VirtualQuery to check

src/trusted/service_runtime/win/sel_memory.c

 /*
  * Copyright (c) 2011 The Native Client Authors. All rights reserved.
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 /*
  * NaCl Service Runtime memory allocation code
  */
 #include "native_client/src/include/portability.h"
 #include "native_client/src/include/win/mman.h"
 
 #include <errno.h>
 #include <windows.h>
 #include <string.h>
 
 #include "native_client/src/shared/platform/nacl_global_secure_random.h"
 #include "native_client/src/shared/platform/nacl_log.h"
 #include "native_client/src/shared/platform/win/xlate_system_error.h"
 
 #include "native_client/src/trusted/service_runtime/nacl_config.h"
 #include "native_client/src/trusted/service_runtime/sel_memory.h"
 #include "native_client/src/trusted/service_runtime/sel_util.h"
 
 #define MSGWIDTH  "25"
 
+#if NACL_ARCH_CPU_32_BITS

[Mark Seaborn, 2011/08/16 17:34:10]

As before.

[bbudge, 2011/08/16 19:46:22]

Done.

+
+int   NaCl_find_sandbox_memory(void   **p,

[Mark Seaborn, 2011/08/16 17:34:10]

Can you put a comment in to say that the purpose of this is to look for a region
reserved by the parent process on our behalf?

[bbudge, 2011/08/16 19:46:22]

Done.

+                               size_t num_bytes) {
+  SYSTEM_INFO sys_info;
+  MEMORY_BASIC_INFORMATION mem;
+  char* start;

[Mark Seaborn, 2011/08/16 17:34:10]

Nit: " *" rather than "* "

[bbudge, 2011/08/16 19:46:22]

Done.

+
+  GetSystemInfo(&sys_info);
+  start = sys_info.lpMinimumApplicationAddress;
+  while (VirtualQuery((LPCVOID)start, &mem, sizeof(mem))) {

[Mark Seaborn, 2011/08/16 17:34:10]

I'd suggest checking that the return value is either 0 or sizeof(mem), just in
case.  i.e. CHECK(result == sizeof(mem)) after.

[bbudge, 2011/08/16 19:46:22]

Done.

+    if (mem.State == MEM_RESERVE && mem.RegionSize == num_bytes) {
+      VirtualFree(start, 0, MEM_RELEASE);

[Mark Seaborn, 2011/08/16 17:34:10]

Please check return value.  You can use LOG_FATAL on error.

[bbudge, 2011/08/16 19:46:22]

Done.

+      *p = start;
+      return 0;
+    }
+    start += mem.RegionSize;
+    if ((LPVOID)start > sys_info.lpMaximumApplicationAddress)

[Mark Seaborn, 2011/08/16 17:34:10]

Minor nit: maybe '>='?

[bbudge, 2011/08/16 19:46:22]

Done.

+      break;
+  }
+  return -ENOMEM;
+}
+
+#endif  /* NACL_ARCH_CPU_32_BITS */
+
 /*
  * NaCl_page_free: free pages allocated with NaCl_page_alloc.
  * Must start at allocation granularity (NACL_MAP_PAGESIZE) and
  * number of bytes must be a multiple of allocation granularity.
  */
 void  NaCl_page_free(void     *p,
                      size_t   num_bytes) {
   void  *end_addr;
 
   end_addr = (void *) (((char *) p) + num_bytes);
   while (p < end_addr) {
     if (!VirtualFree(p, 0, MEM_RELEASE)) {
       DWORD err = GetLastError();
       NaClLog(LOG_FATAL,
               "NaCl_page_free: VirtualFree(0x%016"NACL_PRIxPTR
               ", 0, MEM_RELEASE) failed "
               "with error 0x%X\n",
               (uintptr_t) p, err);
     }
     p = (void *) (((char *) p) + NACL_MAP_PAGESIZE);
   }
 }
 
 
-static
-int   NaCl_page_alloc_hint(void    **p,
+int   NaCl_page_alloc_at_addr(void    **p,
                            size_t  num_bytes) {
   SYSTEM_INFO sys_info;
 
   int         attempt_count;
 
   void        *hint = *p;
   void        *addr;
   void        *end_addr;
   void        *chunk;
   void        *unroll;
@@ skipping 81 matching lines @@
         /* double break */
       }
     }
     NaClLog(2,
             ("NaCl_page_alloc: *p = 0x%016"NACL_PRIxPTR","
              " returning success.\n"),
             (uintptr_t) addr);
     *p = addr;
     return 0;
  retry:
-    NaClLog(2, "NaCl_page_alloc_hint: retrying w/o hint\n");
+    NaClLog(2, "NaCl_page_alloc_at_addr: retrying w/o hint\n");
     hint = NULL;
   }
 
   return -ENOMEM;
 }
 
 int   NaCl_page_alloc(void    **p,
                       size_t  num_bytes) {
   *p = NULL;
-  return NaCl_page_alloc_hint(p, num_bytes);
+  return NaCl_page_alloc_at_addr(p, num_bytes);
 }
 
 /*
  * Pick a "hint" address that is random.
  */
 int NaCl_page_alloc_randomized(void   **p,
                                size_t size) {
   uintptr_t addr;
   int       neg_errno = -ENOMEM;  /* in case we change kNumTries to 0 */
   int       tries;
@@ skipping 21 matching lines @@
      * bits of entropy.
      */
     *p = (void *) ((addr << NACL_MAP_PAGESHIFT)  /* bits [47:16] are random */
                    & ((((uintptr_t) 1) << 43) - 1));  /* now bits [42:16] */
 #else
 # error "where am i?"
 #endif
 
     NaClLog(LOG_INFO, "NaCl_page_alloc_randomized: hint 0x%"NACL_PRIxPTR"\n",
             (uintptr_t) *p);
-    neg_errno = NaCl_page_alloc_hint(p, size);
+    neg_errno = NaCl_page_alloc_at_addr(p, size);
     if (0 == neg_errno) {
       break;
     }
   }
   if (0 != neg_errno) {
     NaClLog(LOG_INFO,
             "NaCl_page_alloc_randomized: failed (%d), dropping hints\n",
             -neg_errno);
     *p = 0;
-    neg_errno = NaCl_page_alloc_hint(p, size);
+    neg_errno = NaCl_page_alloc_at_addr(p, size);
   }
   return neg_errno;
 }
 
 uintptr_t NaClProtectChunkSize(uintptr_t start,
                                uintptr_t end) {
   uintptr_t chunk_end;
 
   chunk_end = NaClRoundAllocPage(start + 1);
   if (chunk_end > end) {
@@ skipping 172 matching lines @@
       break;
     case MADV_NORMAL:
       memset(start, 0, length);
       break;
     default:
       return -EINVAL;
   }
   NaClLog(2, "NaCl_madvise: done\n");
   return 0;
 }