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
This commit is contained in:
Dmitriy Ivanov 2014-05-13 18:34:48 -07:00
parent d597d263bc
commit bc23e530c4
3 changed files with 0 additions and 44 deletions

View File

@ -42,8 +42,6 @@ void LinkerBlockAllocator::init(size_t block_size) {
void* LinkerBlockAllocator::alloc() { void* LinkerBlockAllocator::alloc() {
if (free_block_list_ == nullptr) { if (free_block_list_ == nullptr) {
create_new_page(); create_new_page();
} else {
protect_page(free_block_list_, PROT_READ | PROT_WRITE);
} }
FreeBlockInfo* block_info = reinterpret_cast<FreeBlockInfo*>(free_block_list_); 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); FreeBlockInfo* block_info = reinterpret_cast<FreeBlockInfo*>(block);
protect_page(block_info, PROT_READ | PROT_WRITE);
block_info->next_block = free_block_list_; block_info->next_block = free_block_list_;
block_info->num_free_blocks = 1; block_info->num_free_blocks = 1;
protect_page(block_info, PROT_READ);
free_block_list_ = block_info; 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() { void LinkerBlockAllocator::create_new_page() {
LinkerAllocatorPage* page = reinterpret_cast<LinkerAllocatorPage*>(mmap(nullptr, PAGE_SIZE, LinkerAllocatorPage* page = reinterpret_cast<LinkerAllocatorPage*>(mmap(nullptr, PAGE_SIZE,
PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, 0, 0)); PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, 0, 0));

View File

@ -37,7 +37,6 @@ class LinkerBlockAllocator {
void init(size_t block_size); void init(size_t block_size);
void* alloc(); void* alloc();
void free(void* block); void free(void* block);
void protect_page(void* block, int prot);
void protect_all(int prot); void protect_all(int prot);
private: private:
@ -63,7 +62,6 @@ class LinkerAllocator {
void init() { block_allocator_.init(sizeof(T)); } void init() { block_allocator_.init(sizeof(T)); }
T* alloc() { return reinterpret_cast<T*>(block_allocator_.alloc()); } T* alloc() { return reinterpret_cast<T*>(block_allocator_.alloc()); }
void free(T* t) { block_allocator_.free(t); } 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); } void protect_all(int prot) { block_allocator_.protect_all(prot); }
private: private:
LinkerBlockAllocator block_allocator_; LinkerBlockAllocator block_allocator_;

View File

@ -61,8 +61,6 @@ TEST(linker_allocator, test_nominal) {
ptr1->value = 42; ptr1->value = 42;
allocator.protect_page(ptr1, PROT_READ);
allocator.free(ptr1); allocator.free(ptr1);
allocator.free(ptr2); allocator.free(ptr2);
} }
@ -91,8 +89,6 @@ TEST(linker_allocator, test_larger) {
ASSERT_EQ(ptr1+1, ptr2); ASSERT_EQ(ptr1+1, ptr2);
allocator.protect_page(ptr2, PROT_READ);
// lets allocate until we reach next page. // lets allocate until we reach next page.
size_t n = kPageSize/sizeof(test_struct_larger) + 1 - 2; 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() { static void protect_all() {
LinkerAllocator<test_struct_larger> allocator; LinkerAllocator<test_struct_larger> allocator;
allocator.init(); allocator.init();
@ -155,7 +126,6 @@ static void protect_all() {
TEST(linker_allocator, test_protect) { TEST(linker_allocator, test_protect) {
testing::FLAGS_gtest_death_test_style = "threadsafe"; 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"); ASSERT_EXIT(protect_all(), testing::KilledBySignal(SIGSEGV), "trying to access protected page");
} }