diff options
author | Dmitriy Ivanov <dimitry@google.com> | 2014-05-13 18:34:48 -0700 |
---|---|---|
committer | Dmitriy Ivanov <dimitry@google.com> | 2014-05-13 18:34:48 -0700 |
commit | bc23e530c4db5175a065eeef36553c9c2c78fcf4 (patch) | |
tree | dd0ef72c108a549ef4ac775be8cb4f43034ca29a /linker | |
parent | d597d263bc32422402d4810ce4ec070f0227c2f7 (diff) | |
download | bionic-bc23e530c4db5175a065eeef36553c9c2c78fcf4.zip bionic-bc23e530c4db5175a065eeef36553c9c2c78fcf4.tar.gz bionic-bc23e530c4db5175a065eeef36553c9c2c78fcf4.tar.bz2 |
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
Diffstat (limited to 'linker')
-rw-r--r-- | linker/linker_allocator.cpp | 12 | ||||
-rw-r--r-- | linker/linker_allocator.h | 2 | ||||
-rw-r--r-- | linker/tests/linker_allocator_test.cpp | 30 |
3 files changed, 0 insertions, 44 deletions
diff --git a/linker/linker_allocator.cpp b/linker/linker_allocator.cpp index 805844f..60ce1ea 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 e5b63c5..3af99da 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 ccbdce6..e3a91c5 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"); } |