Skip to content

Commit

Permalink
[PA] Move the uintptr_t/void* line lower to the bottom of Page Allocator
Browse files Browse the repository at this point in the history
There are plenty of reinterpret_casts on the border between
PartitionAlloc and the Page Allocator layer. By transitioning the Page
Allocator to uintptr_t, the reinterpret_casts can be nicely contained
to locations where it interacts with the OS APIs that require void*.

Since the Page Allocator API is public, some of its functions needed to
keep their void* counterpart (side-by-side, as an overloaded function).

Bug: 1280844
Change-Id: I9256c27088ace2b84e4bb8128f9ea8ad195e68ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3396342
Reviewed-by: Benoit Lize <lizeb@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Bartek Nowierski <bartekn@chromium.org>
Auto-Submit: Bartek Nowierski <bartekn@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/main@{#961328}
  • Loading branch information
bartekn-chromium authored and Chromium LUCI CQ committed Jan 20, 2022
1 parent 5e5152d commit bd4cffe
Show file tree
Hide file tree
Showing 23 changed files with 483 additions and 432 deletions.
22 changes: 10 additions & 12 deletions base/allocator/partition_allocator/address_pool_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,24 @@

#include "base/allocator/partition_allocator/address_pool_manager.h"

#include "build/build_config.h"

#if BUILDFLAG(IS_APPLE)
#include <sys/mman.h>
#endif

#include <algorithm>
#include <cstdint>
#include <limits>

#include "base/allocator/buildflags.h"
#include "base/allocator/partition_allocator/page_allocator.h"
#include "base/allocator/partition_allocator/page_allocator_constants.h"
#include "base/allocator/partition_allocator/page_allocator_internal.h"
#include "base/allocator/partition_allocator/partition_alloc_check.h"
#include "base/allocator/partition_allocator/partition_alloc_constants.h"
#include "base/allocator/partition_allocator/partition_alloc_notreached.h"
#include "base/allocator/partition_allocator/reservation_offset_table.h"
#include "base/cxx17_backports.h"
#include "base/lazy_instance.h"
#include "build/build_config.h"

#if BUILDFLAG(IS_APPLE)
#include <sys/mman.h>
#endif

namespace base {
namespace internal {
Expand All @@ -48,7 +47,7 @@ void DecommitPages(uintptr_t address, size_t size) {
// Callers rely on the pages being zero-initialized when recommitting them.
// |DecommitSystemPages| doesn't guarantee this on all operating systems, in
// particular on macOS, but |DecommitAndZeroSystemPages| does.
DecommitAndZeroSystemPages(reinterpret_cast<void*>(address), size);
DecommitAndZeroSystemPages(address, size);
}

} // namespace
Expand Down Expand Up @@ -303,9 +302,8 @@ uintptr_t AddressPoolManager::Reserve(pool_handle handle,
uintptr_t requested_address,
size_t length) {
PA_DCHECK(!(length & DirectMapAllocationGranularityOffsetMask()));
uintptr_t address = reinterpret_cast<uintptr_t>(
AllocPages(reinterpret_cast<void*>(requested_address), length,
kSuperPageSize, PageInaccessible, PageTag::kPartitionAlloc));
uintptr_t address = AllocPages(requested_address, length, kSuperPageSize,
PageInaccessible, PageTag::kPartitionAlloc);
return address;
}

Expand All @@ -314,7 +312,7 @@ void AddressPoolManager::UnreserveAndDecommit(pool_handle handle,
size_t length) {
PA_DCHECK(!(address & kSuperPageOffsetMask));
PA_DCHECK(!(length & DirectMapAllocationGranularityOffsetMask()));
FreePages(reinterpret_cast<void*>(address), length);
FreePages(address, length);
}

void AddressPoolManager::MarkUsed(pool_handle handle,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
// found in the LICENSE file.

#include "base/allocator/partition_allocator/address_pool_manager.h"

#include <cstdint>

#include "base/allocator/partition_allocator/page_allocator.h"
#include "base/allocator/partition_allocator/page_allocator_internal.h"
#include "base/allocator/partition_allocator/partition_alloc_constants.h"
#include "base/bits.h"
#include "build/build_config.h"
Expand All @@ -30,16 +30,16 @@ class PartitionAllocAddressPoolManagerTest : public testing::Test {

void SetUp() override {
manager_ = std::make_unique<AddressPoolManagerForTesting>();
base_ptr_ = AllocPages(nullptr, kPoolSize, kSuperPageSize,
base::PageInaccessible, PageTag::kPartitionAlloc);
base_address_ = reinterpret_cast<uintptr_t>(base_ptr_);
base_address_ =
AllocPages(kPoolSize, kSuperPageSize, base::PageInaccessible,
PageTag::kPartitionAlloc);
ASSERT_TRUE(base_address_);
pool_ = manager_->Add(base_address_, kPoolSize);
}

void TearDown() override {
manager_->Remove(pool_);
FreePages(base_ptr_, kPoolSize);
FreePages(base_address_, kPoolSize);
manager_.reset();
}

Expand All @@ -49,7 +49,6 @@ class PartitionAllocAddressPoolManagerTest : public testing::Test {
static constexpr size_t kPoolSize = kSuperPageSize * kPageCnt;

std::unique_ptr<AddressPoolManagerForTesting> manager_;
void* base_ptr_;
uintptr_t base_address_;
pool_handle pool_;
};
Expand Down Expand Up @@ -196,23 +195,21 @@ TEST_F(PartitionAllocAddressPoolManagerTest, DecommittedDataIsErased) {
uintptr_t address =
GetAddressPoolManager()->Reserve(pool_, 0, kSuperPageSize);
ASSERT_TRUE(address);
void* ptr = reinterpret_cast<void*>(address);
RecommitSystemPages(ptr, kSuperPageSize, PageReadWrite,
RecommitSystemPages(address, kSuperPageSize, PageReadWrite,
PageUpdatePermissions);

memset(ptr, 42, kSuperPageSize);
memset(reinterpret_cast<void*>(address), 42, kSuperPageSize);
GetAddressPoolManager()->UnreserveAndDecommit(pool_, address, kSuperPageSize);

uintptr_t address2 =
GetAddressPoolManager()->Reserve(pool_, 0, kSuperPageSize);
ASSERT_EQ(address, address2);
uint8_t* ptr2 = reinterpret_cast<uint8_t*>(address2);
RecommitSystemPages(ptr2, kSuperPageSize, PageReadWrite,
RecommitSystemPages(address2, kSuperPageSize, PageReadWrite,
PageUpdatePermissions);

uint32_t sum = 0;
for (size_t i = 0; i < kSuperPageSize; i++) {
sum += ptr2[i];
sum += reinterpret_cast<uint8_t*>(address2)[i];
}
EXPECT_EQ(0u, sum) << sum / 42 << " bytes were not zeroed";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "base/allocator/partition_allocator/address_space_randomization.h"

#include <cstdint>

#include "base/allocator/partition_allocator/page_allocator.h"
#include "base/allocator/partition_allocator/partition_alloc_check.h"
#include "base/allocator/partition_allocator/random.h"
Expand All @@ -18,7 +20,7 @@

namespace base {

void* GetRandomPageBase() {
uintptr_t GetRandomPageBase() {
uintptr_t random = static_cast<uintptr_t>(RandomValue());

#if defined(ARCH_CPU_64_BITS)
Expand Down Expand Up @@ -55,14 +57,14 @@ void* GetRandomPageBase() {
if (is_wow64 == -1 && !IsWow64Process(GetCurrentProcess(), &is_wow64))
is_wow64 = FALSE;
if (!is_wow64)
return nullptr;
return 0;
#endif // BUILDFLAG(IS_WIN)
random &= internal::ASLRMask();
random += internal::ASLROffset();
#endif // defined(ARCH_CPU_32_BITS)

PA_DCHECK(!(random & PageAllocationGranularityOffsetMask()));
return reinterpret_cast<void*>(random);
return random;
}

} // namespace base
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef BASE_ALLOCATOR_PARTITION_ALLOCATOR_ADDRESS_SPACE_RANDOMIZATION_H_
#define BASE_ALLOCATOR_PARTITION_ALLOCATOR_ADDRESS_SPACE_RANDOMIZATION_H_

#include <cstdint>

#include "base/allocator/partition_allocator/page_allocator.h"
#include "base/base_export.h"
#include "base/compiler_specific.h"
Expand All @@ -14,7 +16,7 @@ namespace base {

// Calculates a random preferred mapping address. In calculating an address, we
// balance good ASLR against not fragmenting the address space too badly.
BASE_EXPORT void* GetRandomPageBase();
BASE_EXPORT uintptr_t GetRandomPageBase();

namespace internal {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "base/allocator/partition_allocator/address_space_randomization.h"

#include <cstdint>
#include <vector>

#include "base/allocator/partition_allocator/page_allocator.h"
Expand Down Expand Up @@ -48,7 +49,7 @@ uintptr_t GetMask() {
const size_t kSamples = 100;

uintptr_t GetAddressBits() {
return reinterpret_cast<uintptr_t>(base::GetRandomPageBase());
return base::GetRandomPageBase();
}

uintptr_t GetRandomBits() {
Expand All @@ -63,10 +64,10 @@ TEST(PartitionAllocAddressSpaceRandomizationTest, DisabledASLR) {
if (!mask) {
#if BUILDFLAG(IS_WIN) && defined(ARCH_CPU_32_BITS)
// ASLR should be turned off on 32-bit Windows.
EXPECT_EQ(nullptr, base::GetRandomPageBase());
EXPECT_EQ(0u, base::GetRandomPageBase());
#else
// Otherwise, nullptr is very unexpected.
EXPECT_NE(nullptr, base::GetRandomPageBase());
// Otherwise, 0 is very unexpected.
EXPECT_NE(0u, base::GetRandomPageBase());
#endif
}
}
Expand Down Expand Up @@ -106,15 +107,13 @@ TEST(PartitionAllocAddressSpaceRandomizationTest, Predictable) {

std::vector<uintptr_t> sequence;
for (size_t i = 0; i < kSamples; ++i) {
uintptr_t address = reinterpret_cast<uintptr_t>(base::GetRandomPageBase());
sequence.push_back(address);
sequence.push_back(GetRandomPageBase());
}

base::SetMmapSeedForTesting(kInitialSeed);

for (size_t i = 0; i < kSamples; ++i) {
uintptr_t address = reinterpret_cast<uintptr_t>(base::GetRandomPageBase());
EXPECT_EQ(address, sequence[i]);
EXPECT_EQ(GetRandomPageBase(), sequence[i]);
}
}

Expand Down
Loading

0 comments on commit bd4cffe

Please sign in to comment.