From bc23e530c4db5175a065eeef36553c9c2c78fcf4 Mon Sep 17 00:00:00 2001
From: Dmitriy Ivanov <dimitry@google.com>
Date: Tue, 13 May 2014 18:34:48 -0700
Subject: [PATCH] Remove page level mprotects

Freeing block mprotects on the page which it turn
may lead to application crash if linker subsequently
tries to modify another block on the page.

Bug: 14895266
Change-Id: I8ff7f5df467d7be184242de652032b3c84e24b76
---
 linker/linker_allocator.cpp            | 12 -----------
 linker/linker_allocator.h              |  2 --
 linker/tests/linker_allocator_test.cpp | 30 --------------------------
 3 files changed, 44 deletions(-)

diff --git a/linker/linker_allocator.cpp b/linker/linker_allocator.cpp
index 805844fc2..60ce1ea47 100644
--- a/linker/linker_allocator.cpp
+++ b/linker/linker_allocator.cpp
@@ -42,8 +42,6 @@ void LinkerBlockAllocator::init(size_t block_size) {
 void* LinkerBlockAllocator::alloc() {
   if (free_block_list_ == nullptr) {
     create_new_page();
-  } else {
-    protect_page(free_block_list_, PROT_READ | PROT_WRITE);
   }
 
   FreeBlockInfo* block_info = reinterpret_cast<FreeBlockInfo*>(free_block_list_);
@@ -82,10 +80,8 @@ void LinkerBlockAllocator::free(void* block) {
 
   FreeBlockInfo* block_info = reinterpret_cast<FreeBlockInfo*>(block);
 
-  protect_page(block_info, PROT_READ | PROT_WRITE);
   block_info->next_block = free_block_list_;
   block_info->num_free_blocks = 1;
-  protect_page(block_info, PROT_READ);
 
   free_block_list_ = block_info;
 }
@@ -98,14 +94,6 @@ void LinkerBlockAllocator::protect_all(int prot) {
   }
 }
 
-void LinkerBlockAllocator::protect_page(void* block, int prot) {
-  LinkerAllocatorPage* page = find_page(block);
-  if (page == nullptr || mprotect(page, PAGE_SIZE, prot) == -1) {
-    abort();
-  }
-}
-
-
 void LinkerBlockAllocator::create_new_page() {
   LinkerAllocatorPage* page = reinterpret_cast<LinkerAllocatorPage*>(mmap(nullptr, PAGE_SIZE,
       PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, 0, 0));
diff --git a/linker/linker_allocator.h b/linker/linker_allocator.h
index e5b63c593..3af99da76 100644
--- a/linker/linker_allocator.h
+++ b/linker/linker_allocator.h
@@ -37,7 +37,6 @@ class LinkerBlockAllocator {
   void init(size_t block_size);
   void* alloc();
   void free(void* block);
-  void protect_page(void* block, int prot);
   void protect_all(int prot);
 
  private:
@@ -63,7 +62,6 @@ class LinkerAllocator {
   void init() { block_allocator_.init(sizeof(T)); }
   T* alloc() { return reinterpret_cast<T*>(block_allocator_.alloc()); }
   void free(T* t) { block_allocator_.free(t); }
-  void protect_page(T* t, int prot) { block_allocator_.protect_page(t, prot); }
   void protect_all(int prot) { block_allocator_.protect_all(prot); }
  private:
   LinkerBlockAllocator block_allocator_;
diff --git a/linker/tests/linker_allocator_test.cpp b/linker/tests/linker_allocator_test.cpp
index ccbdce6f0..e3a91c59f 100644
--- a/linker/tests/linker_allocator_test.cpp
+++ b/linker/tests/linker_allocator_test.cpp
@@ -61,8 +61,6 @@ TEST(linker_allocator, test_nominal) {
 
   ptr1->value = 42;
 
-  allocator.protect_page(ptr1, PROT_READ);
-
   allocator.free(ptr1);
   allocator.free(ptr2);
 }
@@ -91,8 +89,6 @@ TEST(linker_allocator, test_larger) {
 
   ASSERT_EQ(ptr1+1, ptr2);
 
-  allocator.protect_page(ptr2, PROT_READ);
-
   // lets allocate until we reach next page.
   size_t n = kPageSize/sizeof(test_struct_larger) + 1 - 2;
 
@@ -102,31 +98,6 @@ TEST(linker_allocator, test_larger) {
 
 }
 
-static void protect_one_page() {
-  LinkerAllocator<test_struct_larger> allocator;
-  allocator.init();
-
-  // number of allocs to reach the end of first page
-  size_t n = kPageSize/sizeof(test_struct_larger) - 1;
-  test_struct_larger* page1_ptr = allocator.alloc();
-
-  for (size_t i=0; i<n; ++i) {
-    allocator.alloc();
-  }
-
-  test_struct_larger* page2_ptr = allocator.alloc();
-
-  allocator.protect_page(page2_ptr, PROT_READ);
-
-  // check that we still have access to page1
-  page1_ptr->dummy_str[17] = 52;
-
-  fprintf(stderr, "trying to access protected page");
-
-  // this should result in segmentation fault
-  page2_ptr->dummy_str[12] = 3;
-}
-
 static void protect_all() {
   LinkerAllocator<test_struct_larger> allocator;
   allocator.init();
@@ -155,7 +126,6 @@ static void protect_all() {
 
 TEST(linker_allocator, test_protect) {
   testing::FLAGS_gtest_death_test_style = "threadsafe";
-  ASSERT_EXIT(protect_one_page(), testing::KilledBySignal(SIGSEGV), "trying to access protected page");
   ASSERT_EXIT(protect_all(), testing::KilledBySignal(SIGSEGV), "trying to access protected page");
 }