Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3ca1976f authored by Colin Cross's avatar Colin Cross
Browse files

Validate allocations against mappings 

Bug 120032857 is seeing what appears to be allocations with incorrect
end addresses, leading to a much later crash when it tries to map
a zero page outside the valid virtual address space.  Detect allocations
that extend outside the highest or lowest memory mapping and crash
immediately instead.

Test: memunreachable_test
Bug: 120032857
Change-Id: I9be670a025143e7078360a6bf7a83219279614d9
parent d780dcba

libmemunreachable/HeapWalker.cpp

+12 −0
Original line number Original line Diff line number Diff line
@@ -35,6 +35,13 @@ bool HeapWalker::Allocation(uintptr_t begin, uintptr_t end) { 
    end = begin + 1;

    end = begin + 1;

  }

  }

  Range range{begin, end};

  Range range{begin, end};

  if (valid_mappings_range_.end != 0 &&

      (begin < valid_mappings_range_.begin || end > valid_mappings_range_.end)) {

    MEM_LOG_ALWAYS_FATAL("allocation %p-%p is outside mapping range %p-%p",

                         reinterpret_cast<void*>(begin), reinterpret_cast<void*>(end),

                         reinterpret_cast<void*>(valid_mappings_range_.begin),

                         reinterpret_cast<void*>(valid_mappings_range_.end));

  }

  auto inserted = allocations_.insert(std::pair<Range, AllocationInfo>(range, AllocationInfo{}));

  auto inserted = allocations_.insert(std::pair<Range, AllocationInfo>(range, AllocationInfo{}));

  if (inserted.second) {

  if (inserted.second) {

    valid_allocations_range_.begin = std::min(valid_allocations_range_.begin, begin);

    valid_allocations_range_.begin = std::min(valid_allocations_range_.begin, begin);
@@ -87,6 +94,11 @@ void HeapWalker::RecurseRoot(const Range& root) { 
  }

  }

}

}





void HeapWalker::Mapping(uintptr_t begin, uintptr_t end) {

  valid_mappings_range_.begin = std::min(valid_mappings_range_.begin, begin);

  valid_mappings_range_.end = std::max(valid_mappings_range_.end, end);

}



void HeapWalker::Root(uintptr_t begin, uintptr_t end) {

void HeapWalker::Root(uintptr_t begin, uintptr_t end) {

  roots_.push_back(Range{begin, end});

  roots_.push_back(Range{begin, end});

}

}

libmemunreachable/HeapWalker.h

+4 −0
Original line number Original line Diff line number Diff line
@@ -59,6 +59,8 @@ class HeapWalker { 
        segv_page_count_(0) {

        segv_page_count_(0) {

    valid_allocations_range_.end = 0;

    valid_allocations_range_.end = 0;

    valid_allocations_range_.begin = ~valid_allocations_range_.end;

    valid_allocations_range_.begin = ~valid_allocations_range_.end;

    valid_mappings_range_.end = 0;

    valid_mappings_range_.begin = ~valid_allocations_range_.end;





    segv_handler_.install(

    segv_handler_.install(

        SIGSEGV, [=](ScopedSignalHandler& handler, int signal, siginfo_t* siginfo, void* uctx) {

        SIGSEGV, [=](ScopedSignalHandler& handler, int signal, siginfo_t* siginfo, void* uctx) {
@@ -68,6 +70,7 @@ class HeapWalker { 




  ~HeapWalker() {}

  ~HeapWalker() {}

  bool Allocation(uintptr_t begin, uintptr_t end);

  bool Allocation(uintptr_t begin, uintptr_t end);

  void Mapping(uintptr_t begin, uintptr_t end);

  void Root(uintptr_t begin, uintptr_t end);

  void Root(uintptr_t begin, uintptr_t end);

  void Root(const allocator::vector<uintptr_t>& vals);

  void Root(const allocator::vector<uintptr_t>& vals);
@@ -98,6 +101,7 @@ class HeapWalker { 
  AllocationMap allocations_;

  AllocationMap allocations_;

  size_t allocation_bytes_;

  size_t allocation_bytes_;

  Range valid_allocations_range_;

  Range valid_allocations_range_;

  Range valid_mappings_range_;





  allocator::vector<Range> roots_;

  allocator::vector<Range> roots_;

  allocator::vector<uintptr_t> root_vals_;

  allocator::vector<uintptr_t> root_vals_;

libmemunreachable/MemUnreachable.cpp

+5 −0
Original line number Original line Diff line number Diff line
@@ -87,6 +87,11 @@ bool MemUnreachable::CollectAllocations(const allocator::vector<ThreadInfo>& thr 
                                        const allocator::vector<Mapping>& mappings,

                                        const allocator::vector<Mapping>& mappings,

                                        const allocator::vector<uintptr_t>& refs) {

                                        const allocator::vector<uintptr_t>& refs) {

  MEM_ALOGI("searching process %d for allocations", pid_);

  MEM_ALOGI("searching process %d for allocations", pid_);



  for (auto it = mappings.begin(); it != mappings.end(); it++) {

    heap_walker_.Mapping(it->begin, it->end);

  }



  allocator::vector<Mapping> heap_mappings{mappings};

  allocator::vector<Mapping> heap_mappings{mappings};

  allocator::vector<Mapping> anon_mappings{mappings};

  allocator::vector<Mapping> anon_mappings{mappings};

  allocator::vector<Mapping> globals_mappings{mappings};

  allocator::vector<Mapping> globals_mappings{mappings};

libmemunreachable/tests/HeapWalker_test.cpp

+18 −0
Original line number Original line Diff line number Diff line
@@ -73,6 +73,24 @@ TEST_F(HeapWalkerTest, zero) { 
  ASSERT_FALSE(heap_walker.Allocation(2, 3));

  ASSERT_FALSE(heap_walker.Allocation(2, 3));

}

}





TEST_F(HeapWalkerTest, mapping) {

  HeapWalker heap_walker(heap_);

  heap_walker.Mapping(2, 3);

  heap_walker.Mapping(4, 5);

  ASSERT_TRUE(heap_walker.Allocation(2, 3));

  ASSERT_TRUE(heap_walker.Allocation(4, 5));

  // space between mappings is not checked, but could be in the future

  ASSERT_TRUE(heap_walker.Allocation(3, 4));



  // re-enable malloc, ASSERT_DEATH may allocate

  disable_malloc_.Enable();

  ASSERT_DEATH({ heap_walker.Allocation(1, 2); }, "0x1-0x2.*outside.*0x2-0x5");

  ASSERT_DEATH({ heap_walker.Allocation(1, 3); }, "0x1-0x3.*outside.*0x2-0x5");

  ASSERT_DEATH({ heap_walker.Allocation(4, 6); }, "0x4-0x6.*outside.*0x2-0x5");

  ASSERT_DEATH({ heap_walker.Allocation(5, 6); }, "0x5-0x6.*outside.*0x2-0x5");

  ASSERT_DEATH({ heap_walker.Allocation(1, 6); }, "0x1-0x6.*outside.*0x2-0x5");

}



#define buffer_begin(buffer) reinterpret_cast<uintptr_t>(buffer)

#define buffer_begin(buffer) reinterpret_cast<uintptr_t>(buffer)

#define buffer_end(buffer) (reinterpret_cast<uintptr_t>(buffer) + sizeof(buffer))

#define buffer_end(buffer) (reinterpret_cast<uintptr_t>(buffer) + sizeof(buffer))