Commit e44c745d authored by Clemens Backes's avatar Clemens Backes Committed by V8 LUCI CQ

Reland "[base] Add new API to protect data memory"

This is a reland of commit 9d36b2dd.
The test case is fixed to actually protect a part of the data section
instead of the stack (which was unintended and could lead to segfaults).

Original change's description:
> [base] Add new API to protect data memory
>
> This adds a new {base::OS::SetDataReadOnly} method, which is similar to
> {SetPermissions(kRead)}, but using another system call on Windows such
> that it works on pages in the data segment.
> {VirtualAlloc} will fail if called on a page of the data section,
> whereas {VirtualProtect} succeeds. For the general {SetPermissions}
> API we still want to use {VirtualAlloc} though, as it also changes the "committed" state of the pages.
>
> Note that we do not add a platform API for this, as the memory was
> never allocated through the platform. We just directly protect it in
> V8.
>
> R=mlippautz@chromium.org
>
> Bug: v8:12887
> Change-Id: If83bf6e5c500cc5cf08c76d04dfac5e2b4d35a2d
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3820482
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82349}

Bug: v8:12887
Change-Id: Ib7c24b43b53d568dafb4a56cf8db7479c784e8d8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3825889Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82398}
parent 21af424e
......@@ -292,6 +292,10 @@ bool OS::SetPermissions(void* address, size_t size, MemoryPermission access) {
address, size, access);
}
void OS::SetDataReadOnly(void* address, size_t size) {
CHECK(OS::SetPermissions(address, size, MemoryPermission::kRead));
}
// static
bool OS::RecommitPages(void* address, size_t size, MemoryPermission access) {
return SetPermissions(address, size, access);
......
......@@ -509,6 +509,14 @@ bool OS::SetPermissions(void* address, size_t size, MemoryPermission access) {
return ret == 0;
}
// static
void OS::SetDataReadOnly(void* address, size_t size) {
DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % CommitPageSize());
DCHECK_EQ(0, size % CommitPageSize());
CHECK_EQ(0, mprotect(address, size, PROT_READ));
}
// static
bool OS::RecommitPages(void* address, size_t size, MemoryPermission access) {
DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % CommitPageSize());
......
......@@ -1015,6 +1015,15 @@ bool OS::SetPermissions(void* address, size_t size, MemoryPermission access) {
return result != nullptr;
}
void OS::SetDataReadOnly(void* address, size_t size) {
DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % CommitPageSize());
DCHECK_EQ(0, size % CommitPageSize());
unsigned long old_protection;
CHECK(VirtualProtect(address, size, PAGE_READONLY, &old_protection));
CHECK_EQ(PAGE_READWRITE, old_protection);
}
// static
bool OS::RecommitPages(void* address, size_t size, MemoryPermission access) {
return SetPermissions(address, size, access);
......
......@@ -338,6 +338,9 @@ class V8_BASE_EXPORT OS {
void* new_address,
MemoryPermission access);
// Make part of the process's data memory read-only.
static void SetDataReadOnly(void* address, size_t size);
private:
// These classes use the private memory management API below.
friend class AddressSpaceReservation;
......
......@@ -238,6 +238,27 @@ TEST(StackTest, StackVariableInBounds) {
}
#endif // !V8_OS_FUCHSIA
using SetDataReadOnlyTest = ::testing::Test;
TEST_F(SetDataReadOnlyTest, SetDataReadOnly) {
static struct alignas(kMaxPageSize) TestData {
int x;
int y;
} test_data;
static_assert(alignof(TestData) == kMaxPageSize);
static_assert(sizeof(TestData) == kMaxPageSize);
test_data.x = 25;
test_data.y = 41;
OS::SetDataReadOnly(&test_data, sizeof(test_data));
CHECK_EQ(25, test_data.x);
CHECK_EQ(41, test_data.y);
ASSERT_DEATH_IF_SUPPORTED(test_data.x = 1, "");
ASSERT_DEATH_IF_SUPPORTED(test_data.y = 0, "");
}
} // namespace base
namespace {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment