Enable the service runtime to use a zero-based sandbox on Linux.

src/trusted/service_runtime/arch/x86_32/sel_addrspace_x86_32.c

Index: src/trusted/service_runtime/arch/x86_32/sel_addrspace_x86_32.c
diff --git a/src/trusted/service_runtime/arch/x86_32/sel_addrspace_x86_32.c b/src/trusted/service_runtime/arch/x86_32/sel_addrspace_x86_32.c
index 8cd92a7f65ac7e0af13ff80f6d0f496e22f41e84..424d179cd985ef2d10df2f9ca2c3c7f0722c9dd5 100644
--- a/src/trusted/service_runtime/arch/x86_32/sel_addrspace_x86_32.c
+++ b/src/trusted/service_runtime/arch/x86_32/sel_addrspace_x86_32.c
@@ -11,21 +11,43 @@
 
 
 NaClErrorCode NaClAllocateSpace(void **mem, size_t addrsp_size) {
+  const void* ONE_MEGABYTE = (void *)(1024*1024);
   int result;
 
   CHECK(NULL != mem);
 
-#ifdef NACL_SANDBOX_FIXED_AT_ZERO
+#if NACL_LINUX && NACL_BUILD_SUBARCH == 32
   /*
-   * When creating a zero-based sandbox, we do not allocate the first 64K of
-   * pages beneath the trampolines, because -- on Linux at least -- we cannot.
-   * Instead, we allocate starting at the trampolines, and then coerce the
-   * out parameter.
+   * On 32 bit Linux, a 1 gigabyte block of address space may be reserved at
+   * the zero-end of the address space during process creation, to address
+   * sandbox layout requirements on ARM and performance issues on Intel ATOM.
+   * Look for this pre-reserved block and if found, pass its address to the
+   * page allocation function.
    */
-  addrsp_size -= NACL_TRAMPOLINE_START;
-  *mem = (void *) NACL_TRAMPOLINE_START;
-  result = NaCl_page_alloc_at_addr(mem, addrsp_size);
-  *mem = 0;
+  if (!NaCl_find_prereserved_sandbox_memory(mem, addrsp_size)) {

[Mark Seaborn, 2011/08/18 23:09:45]

It would be more readable to put the success branch for this function call
first.

+    /* zero-based sandbox not pre-reserved */
+    result = NaCl_page_alloc(mem, addrsp_size);
+  } else {
+    /* sanity check zero sandbox base address */
+    if (0 == *mem || ONE_MEGABYTE < *mem) {

[Mark Seaborn, 2011/08/18 23:09:45]

Why are you checking for 1MB?  What's the significance of this value?

[Brad Chen, 2011/08/19 00:24:35]

I've added a better explanation in the comment.

On 2011/08/18 23:09:45, Mark Seaborn wrote:

+      NaClLog(LOG_ERROR, ("NaClAllocateSpace:"
+                          "Can't handle sandbox at high address"
+                          "0x%08"NACL_PRIxPTR"\n",
+                          *mem);
+      return LOAD_NO_MEMORY;
+    }
+
+    /*
+     * When creating a zero-based sandbox, we do not allocate the first 64K of
+     * pages beneath the trampolines, because -- on Linux at least -- we cannot.
+     * Instead, we allocate starting at the trampolines, and then coerce the
+     * "mem" out parameter.
+     */
+    addrsp_size -= NACL_TRAMPOLINE_START;
+    *mem = (void *) NACL_TRAMPOLINE_START;
+    result = NaCl_page_alloc_at_addr(mem, addrsp_size);
+    *mem = 0;
+  }
 #elif NACL_WINDOWS && NACL_BUILD_SUBARCH == 32
   /*
    * On 32 bit Windows, a 1 gigabyte block of address space is reserved before
@@ -59,6 +81,7 @@ NaClErrorCode NaClAllocateSpace(void **mem, size_t addrsp_size) {
 
 NaClErrorCode NaClMprotectGuards(struct NaClApp *nap) {
   uintptr_t start_addr;
+  uintptr_t page_addr;
   int       err;
 
   start_addr = nap->mem_start;
@@ -68,16 +91,33 @@ NaClErrorCode NaClMprotectGuards(struct NaClApp *nap) {
            "size 0x%08x, end 0x%08"NACL_PRIxPTR"\n"),
           start_addr, NACL_SYSCALL_START_ADDR,
           start_addr + NACL_SYSCALL_START_ADDR);
-  if ((err = NaCl_mprotect((void *) start_addr,
-                           NACL_SYSCALL_START_ADDR,
-                           PROT_NONE)) != 0) {
-    NaClLog(LOG_ERROR, ("NaClMemoryProtection: "
-                        "NaCl_mprotect(0x%08"NACL_PRIxPTR", "
-                        "0x%08x, 0x%x) failed, "
-                        "error %d (NULL pointer guard page)\n"),
-            start_addr, NACL_SYSCALL_START_ADDR, PROT_NONE,
-            err);
-    return LOAD_MPROTECT_FAIL;
+    if (0 == start_addr) {

[Mark Seaborn, 2011/08/18 23:09:45]

Wrong indentation

[Brad Chen, 2011/08/19 00:24:35]

Oops! Fixed.

On 2011/08/18 23:09:45, Mark Seaborn wrote:

+      /* Attempt to protect one page at a time. It is normal for  */

[Mark Seaborn, 2011/08/18 23:09:45]

Use the commenting style:
/*
 * ...
 */

[Brad Chen, 2011/08/19 00:24:35]

Done.

+      /* these attempts to fail if the page is already protected. */

[Mark Seaborn, 2011/08/18 23:09:45]

I don't understand the purpose of this block of code.

What is the state of the bottom 64k of address space before and after this
block?  Why is it OK to ignore errors here?

[Brad Chen, 2011/08/19 00:24:35]

Sounds like the comment above didn't adequate explain things for you. Let's talk
F2F so I can figure out what would.

On 2011/08/18 23:09:45, Mark Seaborn wrote:

+      for (page_addr = 0; page_addr < NACL_SYSCALL_START_ADDR;
+           page_addr += NACL_PAGESIZE) {
+        if ((err = NaCl_mprotect((void *) page_addr, NACL_PAGESIZE,
+                                 PROT_NONE)) != 0) {
+          NaClLog(4, ("NaClMemoryProtection: "
+                      "NaCl_mprotect(0x%08"NACL_PRIxPTR", "
+                      "0x%08x, 0x%x) failed, "
+                      "error %d (NULL pointer guard page)\n"),
+                  page_addr, NACL_PAGESIZE, PROT_NONE,
+                  err);
+        }
+      }
+    } else {
+      if ((err = NaCl_mprotect((void *) start_addr,
+                               NACL_SYSCALL_START_ADDR,
+                               PROT_NONE)) != 0) {
+      NaClLog(LOG_ERROR, ("NaClMemoryProtection: "

[Mark Seaborn, 2011/08/18 23:09:45]

This is not indented from the "if"

[Brad Chen, 2011/08/19 00:24:35]

Done.

+                          "NaCl_mprotect(0x%08"NACL_PRIxPTR", "
+                          "0x%08x, 0x%x) failed, "
+                          "error %d (NULL pointer guard page)\n"),
+              start_addr, NACL_SYSCALL_START_ADDR, PROT_NONE,
+              err);
+      return LOAD_MPROTECT_FAIL;
+    }
   }
   if (!NaClVmmapAdd(&nap->mem_map,
                     (start_addr - nap->mem_start) >> NACL_PAGESHIFT,