From aa2bc55777b96af325fe5071a8a58efe2e6a4ff5 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Thu, 30 Mar 2017 22:39:39 -0400 Subject: [PATCH] posix: Allow ScopedMmap::ResetAddrLen() to deal with overlap It should be possible to shrink a region already supervised by ScopedMmap, or in rare cases when ScopedMmap is supervising only a smaller portion of an overall larger region, increase the size of the region it supervises. This is now equivalent to the operation of base::mac::ScopedMachVM::reset(). The Reset() and ResetAddrLen() methods are upgraded from a void return to a bool return to indicate their success. Bug: crashpad:30 Test: crashpad_util_test ScopedMmap*.ResetAddrLen_* Change-Id: I564e154cd2387e8df3f83b416ecc1c83c9bcf71d Reviewed-on: https://chromium-review.googlesource.com/464286 Commit-Queue: Mark Mentovai Reviewed-by: Joshua Peraza --- util/posix/scoped_mmap.cc | 62 +++++++++++-- util/posix/scoped_mmap.h | 13 ++- util/posix/scoped_mmap_test.cc | 163 ++++++++++++++++++++++++++++++--- 3 files changed, 213 insertions(+), 25 deletions(-) diff --git a/util/posix/scoped_mmap.cc b/util/posix/scoped_mmap.cc index 8c0ba3ee..f8449b1f 100644 --- a/util/posix/scoped_mmap.cc +++ b/util/posix/scoped_mmap.cc @@ -14,27 +14,67 @@ #include "util/posix/scoped_mmap.h" +#include + +#include + #include "base/logging.h" +namespace { + +bool Munmap(uintptr_t addr, size_t len) { + if (munmap(reinterpret_cast(addr), len) != 0) { + PLOG(ERROR) << "munmap"; + return false; + } + + return true; +} + +} // namespace + namespace crashpad { ScopedMmap::ScopedMmap() {} ScopedMmap::~ScopedMmap() { - Reset(); + if (is_valid()) { + Munmap(reinterpret_cast(addr_), len_); + } } -void ScopedMmap::Reset() { - ResetAddrLen(MAP_FAILED, 0); +bool ScopedMmap::Reset() { + return ResetAddrLen(MAP_FAILED, 0); } -void ScopedMmap::ResetAddrLen(void* addr, size_t len) { - if (is_valid() && munmap(addr_, len_) != 0) { - LOG(ERROR) << "munmap"; +bool ScopedMmap::ResetAddrLen(void* addr, size_t len) { + const uintptr_t new_addr = reinterpret_cast(addr); + + if (addr == MAP_FAILED) { + DCHECK_EQ(len, 0u); + } else { + DCHECK_NE(len, 0u); + DCHECK_EQ(new_addr % getpagesize(), 0u); + DCHECK_EQ(len % getpagesize(), 0u); + } + + bool result = true; + + if (is_valid()) { + const uintptr_t old_addr = reinterpret_cast(addr_); + if (old_addr < new_addr) { + result &= Munmap(old_addr, std::min(len_, new_addr - old_addr)); + } + if (old_addr + len_ > new_addr + len) { + uintptr_t unmap_start = std::max(old_addr, new_addr + len); + result &= Munmap(unmap_start, old_addr + len_ - unmap_start); + } } addr_ = addr; len_ = len; + + return result; } bool ScopedMmap::ResetMmap(void* addr, @@ -43,21 +83,27 @@ bool ScopedMmap::ResetMmap(void* addr, int flags, int fd, off_t offset) { + // Reset() first, so that a new anonymous mapping can use the address space + // occupied by the old mapping if appropriate. The new mapping will be + // attempted even if there was something wrong with the old mapping, so don’t + // consider the return value from Reset(). Reset(); void* new_addr = mmap(addr, len, prot, flags, fd, offset); if (new_addr == MAP_FAILED) { - LOG(ERROR) << "mmap"; + PLOG(ERROR) << "mmap"; return false; } + // The new mapping is effective even if there was something wrong with the old + // mapping, so don’t consider the return value from ResetAddrLen(). ResetAddrLen(new_addr, len); return true; } bool ScopedMmap::Mprotect(int prot) { if (mprotect(addr_, len_, prot) < 0) { - LOG(ERROR) << "mprotect"; + PLOG(ERROR) << "mprotect"; return false; } diff --git a/util/posix/scoped_mmap.h b/util/posix/scoped_mmap.h index 51025775..5b216dee 100644 --- a/util/posix/scoped_mmap.h +++ b/util/posix/scoped_mmap.h @@ -33,19 +33,22 @@ class ScopedMmap { //! \brief Releases the memory-mapped region by calling `munmap()`. //! - //! A message will be logged on failure. - void Reset(); + //! \return `true` on success. `false` on failure, with a message logged. + bool Reset(); //! \brief Releases any existing memory-mapped region and sets the object to //! maintain an already-established mapping. //! + //! If \a addr and \a len indicate a region that overlaps with the existing + //! memory-mapped region, only the portion of the existing memory-mapped + //! region that does not overlap the new region, if any, will be released. + //! //! \param[in] addr The base address of the existing memory-mapped region to //! maintain. //! \param[in] len The size of the existing memory-mapped region to maintain. //! - //! A message will be logged on failure to release any existing memory-mapped - //! region, but the new mapping will be set regardless. - void ResetAddrLen(void* addr, size_t len); + //! \return `true` on success. `false` on failure, with a message logged. + bool ResetAddrLen(void* addr, size_t len); //! \brief Releases any existing memory-mapped region and establishes a new //! one by calling `mmap()`. diff --git a/util/posix/scoped_mmap_test.cc b/util/posix/scoped_mmap_test.cc index fa969906..ea96c65f 100644 --- a/util/posix/scoped_mmap_test.cc +++ b/util/posix/scoped_mmap_test.cc @@ -20,6 +20,7 @@ #include "base/numerics/safe_conversions.h" #include "base/rand_util.h" +#include "base/strings/stringprintf.h" #include "gtest/gtest.h" namespace crashpad { @@ -31,17 +32,24 @@ bool ScopedMmapResetMmap(ScopedMmap* mapping, size_t len) { nullptr, len, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); } +void* BareMmap(size_t len) { + return mmap( + nullptr, len, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0); +} + // A weird class. This is used to test that memory-mapped regions are freed // as expected by calling munmap(). This is difficult to test well because once // a region has been unmapped, the address space it formerly occupied becomes // eligible for reuse. // -// The strategy taken here is that a 64-bit cookie value is written into a -// mapped region by SetUp(). While the mapping is active, Check() should -// succeed. After the region is unmapped, calling Check() should fail, either -// because the region has been unmapped and the address not reused, the address -// has been reused but is protected against reading (unlikely), or because the -// address has been reused but the cookie value is no longer present there. +// The strategy taken here is that a random 64-bit cookie value is written into +// a mapped region by SetUp(). While the mapping is active, Check() should not +// crash, or for a gtest expectation, Expected() and Observed() should not crash +// and should be equal. After the region is unmapped, Check() should crash, +// either because the region has been unmapped and the address not reused, the +// address has been reused but is protected against reading (unlikely), or +// because the address has been reused but the cookie value is no longer present +// there. class TestCookie { public: // A weird constructor for a weird class. The member variable initialization @@ -56,8 +64,11 @@ class TestCookie { *address_ = cookie_; } - void Check() { - if (*address_ != cookie_) { + uint64_t Expected() const { return cookie_; } + uint64_t Observed() const { return *address_; } + + void Check() const { + if (Observed() != Expected()) { __builtin_trap(); } } @@ -77,7 +88,7 @@ TEST(ScopedMmap, Mmap) { EXPECT_EQ(MAP_FAILED, mapping.addr()); EXPECT_EQ(0u, mapping.len()); - mapping.Reset(); + ASSERT_TRUE(mapping.Reset()); EXPECT_FALSE(mapping.is_valid()); const size_t kPageSize = base::checked_cast(getpagesize()); @@ -87,9 +98,9 @@ TEST(ScopedMmap, Mmap) { EXPECT_EQ(kPageSize, mapping.len()); cookie.SetUp(mapping.addr_as()); - cookie.Check(); + EXPECT_EQ(cookie.Expected(), cookie.Observed()); - mapping.Reset(); + ASSERT_TRUE(mapping.Reset()); EXPECT_FALSE(mapping.is_valid()); } @@ -122,11 +133,139 @@ TEST(ScopedMmapDeathTest, Reset) { TestCookie cookie; cookie.SetUp(mapping.addr_as()); - mapping.Reset(); + ASSERT_TRUE(mapping.Reset()); EXPECT_DEATH(cookie.Check(), ""); } +TEST(ScopedMmapDeathTest, ResetAddrLen_Shrink) { + ScopedMmap mapping; + + // Start with three pages mapped. + const size_t kPageSize = base::checked_cast(getpagesize()); + ASSERT_TRUE(ScopedMmapResetMmap(&mapping, 3 * kPageSize)); + EXPECT_TRUE(mapping.is_valid()); + EXPECT_NE(MAP_FAILED, mapping.addr()); + EXPECT_EQ(3 * kPageSize, mapping.len()); + + TestCookie cookies[3]; + for (size_t index = 0; index < arraysize(cookies); ++index) { + cookies[index].SetUp(reinterpret_cast( + mapping.addr_as() + index * kPageSize)); + } + + // Reset to the second page. The first and third pages should be unmapped. + void* const new_addr = + reinterpret_cast(mapping.addr_as() + kPageSize); + ASSERT_TRUE(mapping.ResetAddrLen(new_addr, kPageSize)); + EXPECT_TRUE(mapping.is_valid()); + EXPECT_EQ(new_addr, mapping.addr()); + EXPECT_EQ(kPageSize, mapping.len()); + + EXPECT_EQ(cookies[1].Expected(), cookies[1].Observed()); + + EXPECT_DEATH(cookies[0].Check(), ""); + EXPECT_DEATH(cookies[2].Check(), ""); +} + +TEST(ScopedMmap, ResetAddrLen_Grow) { + // Start with three pages mapped, but ScopedMmap only aware of the the second + // page. + const size_t kPageSize = base::checked_cast(getpagesize()); + void* pages = BareMmap(3 * kPageSize); + ASSERT_NE(MAP_FAILED, pages); + + ScopedMmap mapping; + void* const old_addr = + reinterpret_cast(reinterpret_cast(pages) + kPageSize); + ASSERT_TRUE(mapping.ResetAddrLen(old_addr, kPageSize)); + EXPECT_TRUE(mapping.is_valid()); + EXPECT_EQ(old_addr, mapping.addr()); + EXPECT_EQ(kPageSize, mapping.len()); + + TestCookie cookies[3]; + for (size_t index = 0; index < arraysize(cookies); ++index) { + cookies[index].SetUp(reinterpret_cast( + reinterpret_cast(pages) + index * kPageSize)); + } + + // Reset to all three pages. Nothing should be unmapped until destruction. + ASSERT_TRUE(mapping.ResetAddrLen(pages, 3 * kPageSize)); + EXPECT_TRUE(mapping.is_valid()); + EXPECT_EQ(pages, mapping.addr()); + EXPECT_EQ(3 * kPageSize, mapping.len()); + + for (size_t index = 0; index < arraysize(cookies); ++index) { + SCOPED_TRACE(base::StringPrintf("index %zu", index)); + EXPECT_EQ(cookies[index].Expected(), cookies[index].Observed()); + } +} + +TEST(ScopedMmapDeathTest, ResetAddrLen_MoveDownAndGrow) { + // Start with three pages mapped, but ScopedMmap only aware of the third page. + const size_t kPageSize = base::checked_cast(getpagesize()); + void* pages = BareMmap(3 * kPageSize); + ASSERT_NE(MAP_FAILED, pages); + + ScopedMmap mapping; + void* const old_addr = reinterpret_cast( + reinterpret_cast(pages) + 2 * kPageSize); + ASSERT_TRUE(mapping.ResetAddrLen(old_addr, kPageSize)); + EXPECT_TRUE(mapping.is_valid()); + EXPECT_EQ(old_addr, mapping.addr()); + EXPECT_EQ(kPageSize, mapping.len()); + + TestCookie cookies[3]; + for (size_t index = 0; index < arraysize(cookies); ++index) { + cookies[index].SetUp(reinterpret_cast( + reinterpret_cast(pages) + index * kPageSize)); + } + + // Reset to the first two pages. The third page should be unmapped. + ASSERT_TRUE(mapping.ResetAddrLen(pages, 2 * kPageSize)); + EXPECT_TRUE(mapping.is_valid()); + EXPECT_EQ(pages, mapping.addr()); + EXPECT_EQ(2 * kPageSize, mapping.len()); + + EXPECT_EQ(cookies[0].Expected(), cookies[0].Observed()); + EXPECT_EQ(cookies[1].Expected(), cookies[1].Observed()); + + EXPECT_DEATH(cookies[2].Check(), ""); +} + +TEST(ScopedMmapDeathTest, ResetAddrLen_MoveUpAndShrink) { + // Start with three pages mapped, but ScopedMmap only aware of the first two + // pages. + const size_t kPageSize = base::checked_cast(getpagesize()); + void* pages = BareMmap(3 * kPageSize); + ASSERT_NE(MAP_FAILED, pages); + + ScopedMmap mapping; + ASSERT_TRUE(mapping.ResetAddrLen(pages, 2 * kPageSize)); + EXPECT_TRUE(mapping.is_valid()); + EXPECT_EQ(pages, mapping.addr()); + EXPECT_EQ(2 * kPageSize, mapping.len()); + + TestCookie cookies[3]; + for (size_t index = 0; index < arraysize(cookies); ++index) { + cookies[index].SetUp(reinterpret_cast( + reinterpret_cast(pages) + index * kPageSize)); + } + + // Reset to the third page. The first two pages should be unmapped. + void* const new_addr = + reinterpret_cast(mapping.addr_as() + 2 * kPageSize); + ASSERT_TRUE(mapping.ResetAddrLen(new_addr, kPageSize)); + EXPECT_TRUE(mapping.is_valid()); + EXPECT_EQ(new_addr, mapping.addr()); + EXPECT_EQ(kPageSize, mapping.len()); + + EXPECT_EQ(cookies[2].Expected(), cookies[2].Observed()); + + EXPECT_DEATH(cookies[0].Check(), ""); + EXPECT_DEATH(cookies[1].Check(), ""); +} + TEST(ScopedMmapDeathTest, ResetMmap) { ScopedMmap mapping;