Change order of creation of JIT mappings.

To ensure we don't create mappings that can later be turned into
writable mappings.

Ignore-AOSP-First: b/200284993

Test: jit_memory_region_test
Test: device booting
Bug: 200284993
Change-Id: I5160e61d287207563af57b71d50b08364ca892a0

2 files changed, 130 insertions, 62 deletions

diff --git a/runtime/jit/jit_memory_region.cc b/runtime/jit/jit_memory_region.cc
index 58a4041abe..b0699c42d3 100644
--- a/runtime/jit/jit_memory_region.cc
+++ b/runtime/jit/jit_memory_region.cc
@@ -20,6 +20,7 @@
 #include <unistd.h>
 
 #include <android-base/unique_fd.h>
+#include <log/log.h>
 #include "base/bit_utils.h"  // For RoundDown, RoundUp
 #include "base/globals.h"
 #include "base/logging.h"  // For VLOG.
@@ -64,7 +65,15 @@ bool JitMemoryRegion::Initialize(size_t initial_capacity,
   // File descriptor enabling dual-view mapping of code section.
   unique_fd mem_fd;
 
+
+  // The memory mappings we are going to create.
+  MemMap data_pages;
+  MemMap exec_pages;
+  MemMap non_exec_pages;
+  MemMap writable_data_pages;
+
   if (is_zygote) {
+    android_errorWriteLog(0x534e4554, "200284993");  // Report to SafetyNet.
     // Because we are not going to GC code generated by the zygote, just use all available.
     current_capacity_ = max_capacity;
     mem_fd = unique_fd(CreateZygoteMemory(capacity, error_msg));
@@ -92,17 +101,12 @@ bool JitMemoryRegion::Initialize(size_t initial_capacity,
     }
   }
 
+  // Map name specific for android_os_Debug.cpp accounting.
   std::string data_cache_name = is_zygote ? "zygote-data-code-cache" : "data-code-cache";
   std::string exec_cache_name = is_zygote ? "zygote-jit-code-cache" : "jit-code-cache";
 
   std::string error_str;
-  // Map name specific for android_os_Debug.cpp accounting.
-  // Map in low 4gb to simplify accessing root tables for x86_64.
-  // We could do PC-relative addressing to avoid this problem, but that
-  // would require reserving code and data area before submitting, which
-  // means more windows for the code memory to be RWX.
   int base_flags;
-  MemMap data_pages;
   if (mem_fd.get() >= 0) {
     // Dual view of JIT code cache case. Create an initial mapping of data pages large enough
     // for data and non-writable view of JIT code pages. We use the memory file descriptor to
@@ -131,7 +135,63 @@ bool JitMemoryRegion::Initialize(size_t initial_capacity,
     // Additionally, the zyzote will create a dual view of the data portion of
     // the cache. This mapping will be read-only, whereas the second mapping
     // will be writable.
+
     base_flags = MAP_SHARED;
+
+    // Create the writable mappings now, so that in case of the zygote, we can
+    // prevent any future writable mappings through sealing.
+    if (exec_capacity > 0) {
+      // For dual view, create the secondary view of code memory used for updating code. This view
+      // is never executable.
+      std::string name = exec_cache_name + "-rw";
+      non_exec_pages = MemMap::MapFile(exec_capacity,
+                                       kIsDebugBuild ? kProtR : kProtRW,
+                                       base_flags,
+                                       mem_fd,
+                                       /* start= */ data_capacity,
+                                       /* low_4GB= */ false,
+                                       name.c_str(),
+                                       &error_str);
+      if (!non_exec_pages.IsValid()) {
+        // This is unexpected.
+        *error_msg = "Failed to map non-executable view of JIT code cache";
+        return false;
+      }
+      // Create a dual view of the data cache.
+      name = data_cache_name + "-rw";
+      writable_data_pages = MemMap::MapFile(data_capacity,
+                                            kProtRW,
+                                            base_flags,
+                                            mem_fd,
+                                            /* start= */ 0,
+                                            /* low_4GB= */ false,
+                                            name.c_str(),
+                                            &error_str);
+      if (!writable_data_pages.IsValid()) {
+        std::ostringstream oss;
+        oss << "Failed to create dual data view: " << error_str;
+        *error_msg = oss.str();
+        return false;
+      }
+      if (writable_data_pages.MadviseDontFork() != 0) {
+        *error_msg = "Failed to MadviseDontFork the writable data view";
+        return false;
+      }
+      if (non_exec_pages.MadviseDontFork() != 0) {
+        *error_msg = "Failed to MadviseDontFork the writable code view";
+        return false;
+      }
+      // Now that we have created the writable and executable mappings, prevent creating any new
+      // ones.
+      if (is_zygote && !ProtectZygoteMemory(mem_fd.get(), error_msg)) {
+        return false;
+      }
+    }
+
+    // Map in low 4gb to simplify accessing root tables for x86_64.
+    // We could do PC-relative addressing to avoid this problem, but that
+    // would require reserving code and data area before submitting, which
+    // means more windows for the code memory to be RWX.
     data_pages = MemMap::MapFile(
         data_capacity + exec_capacity,
         kProtR,
@@ -172,9 +232,6 @@ bool JitMemoryRegion::Initialize(size_t initial_capacity,
     return false;
   }
 
-  MemMap exec_pages;
-  MemMap non_exec_pages;
-  MemMap writable_data_pages;
   if (exec_capacity > 0) {
     uint8_t* const divider = data_pages.Begin() + data_capacity;
     // Set initial permission for executable view to catch any SELinux permission problems early
@@ -193,59 +250,6 @@ bool JitMemoryRegion::Initialize(size_t initial_capacity,
       *error_msg = oss.str();
       return false;
     }
-
-    if (mem_fd.get() >= 0) {
-      // For dual view, create the secondary view of code memory used for updating code. This view
-      // is never executable.
-      std::string name = exec_cache_name + "-rw";
-      non_exec_pages = MemMap::MapFile(exec_capacity,
-                                       kIsDebugBuild ? kProtR : kProtRW,
-                                       base_flags,
-                                       mem_fd,
-                                       /* start= */ data_capacity,
-                                       /* low_4GB= */ false,
-                                       name.c_str(),
-                                       &error_str);
-      if (!non_exec_pages.IsValid()) {
-        static const char* kFailedNxView = "Failed to map non-executable view of JIT code cache";
-        if (rwx_memory_allowed) {
-          // Log and continue as single view JIT (requires RWX memory).
-          VLOG(jit) << kFailedNxView;
-        } else {
-          *error_msg = kFailedNxView;
-          return false;
-        }
-      }
-      // Create a dual view of the data cache.
-      name = data_cache_name + "-rw";
-      writable_data_pages = MemMap::MapFile(data_capacity,
-                                            kProtRW,
-                                            base_flags,
-                                            mem_fd,
-                                            /* start= */ 0,
-                                            /* low_4GB= */ false,
-                                            name.c_str(),
-                                            &error_str);
-      if (!writable_data_pages.IsValid()) {
-        std::ostringstream oss;
-        oss << "Failed to create dual data view: " << error_str;
-        *error_msg = oss.str();
-        return false;
-      }
-      if (writable_data_pages.MadviseDontFork() != 0) {
-        *error_msg = "Failed to madvise dont fork the writable data view";
-        return false;
-      }
-      if (non_exec_pages.MadviseDontFork() != 0) {
-        *error_msg = "Failed to madvise dont fork the writable code view";
-        return false;
-      }
-      // Now that we have created the writable and executable mappings, prevent creating any new
-      // ones.
-      if (is_zygote && !ProtectZygoteMemory(mem_fd.get(), error_msg)) {
-        return false;
-      }
-    }
   } else {
     // Profiling only. No memory for code required.
   }
diff --git a/runtime/jit/jit_memory_region_test.cc b/runtime/jit/jit_memory_region_test.cc
index 20496118db..18f34fb229 100644
--- a/runtime/jit/jit_memory_region_test.cc
+++ b/runtime/jit/jit_memory_region_test.cc
@@ -492,6 +492,62 @@ class TestZygoteMemory : public testing::Test {
     munmap(addr, kPageSize);
     munmap(shared, kPageSize);
   }
+
+  // Test that a readable mapping created befire sealing future writes, can be
+  // changed into a writable mapping.
+  void TestVmMayWriteBefore() {
+    // Zygote JIT memory only works on kernels that don't segfault on flush.
+    TEST_DISABLED_FOR_KERNELS_WITH_CACHE_SEGFAULT();
+    std::string error_msg;
+    size_t size = kPageSize;
+    int32_t* addr = nullptr;
+    {
+      android::base::unique_fd fd(JitMemoryRegion::CreateZygoteMemory(size, &error_msg));
+      CHECK_NE(fd.get(), -1);
+
+      // Create a shared readable mapping.
+      addr = reinterpret_cast<int32_t*>(
+          mmap(nullptr, kPageSize, PROT_READ, MAP_SHARED, fd.get(), 0));
+      CHECK(addr != nullptr);
+      CHECK_NE(addr, MAP_FAILED);
+
+      // Protect the memory.
+      bool res = JitMemoryRegion::ProtectZygoteMemory(fd.get(), &error_msg);
+      CHECK(res);
+    }
+    // At this point, the fd has been dropped, but the memory mappings are still
+    // there.
+    int res = mprotect(addr, kPageSize, PROT_WRITE);
+    CHECK_EQ(res, 0);
+  }
+
+  // Test that we cannot create a writable mapping after sealing future writes.
+  void TestVmMayWriteAfter() {
+    // Zygote JIT memory only works on kernels that don't segfault on flush.
+    TEST_DISABLED_FOR_KERNELS_WITH_CACHE_SEGFAULT();
+    std::string error_msg;
+    size_t size = kPageSize;
+    int32_t* addr = nullptr;
+    {
+      android::base::unique_fd fd(JitMemoryRegion::CreateZygoteMemory(size, &error_msg));
+      CHECK_NE(fd.get(), -1);
+
+      // Protect the memory.
+      bool res = JitMemoryRegion::ProtectZygoteMemory(fd.get(), &error_msg);
+      CHECK(res);
+
+      // Create a shared readable mapping.
+      addr = reinterpret_cast<int32_t*>(
+          mmap(nullptr, kPageSize, PROT_READ, MAP_SHARED, fd.get(), 0));
+      CHECK(addr != nullptr);
+      CHECK_NE(addr, MAP_FAILED);
+    }
+    // At this point, the fd has been dropped, but the memory mappings are still
+    // there.
+    int res = mprotect(addr, kPageSize, PROT_WRITE);
+    CHECK_EQ(res, -1);
+    CHECK_EQ(errno, EACCES);
+  }
 };
 
 TEST_F(TestZygoteMemory, BasicTest) {
@@ -510,6 +566,14 @@ TEST_F(TestZygoteMemory, TestFromSharedToPrivate) {
   TestFromSharedToPrivate();
 }
 
+TEST_F(TestZygoteMemory, TestVmMayWriteBefore) {
+  TestVmMayWriteBefore();
+}
+
+TEST_F(TestZygoteMemory, TestVmMayWriteAfter) {
+  TestVmMayWriteAfter();
+}
+
 #endif  // defined (__BIONIC__)
 
 }  // namespace jit