Commit db7bdf48 authored by Michael Starzinger's avatar Michael Starzinger Committed by Commit Bot

Revert "[platform] Remove {PageAllocator::kReadWriteExecute}."

This reverts commit bf19e60c.

Reason for revert: Two issues discovered with W^X in V8's 6.5 branch (see v8:7272 and chromium:793428). Still need a way to disable the feature.

Original change's description:
> [platform] Remove {PageAllocator::kReadWriteExecute}.
> 
> Now that write-protection of code memory is enabled everywhere and V8 is
> fully W^X compliant, we can remove the permission mode in question.
> 
> R=​hpayer@chromium.org
> BUG=v8:6792
> 
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
> Change-Id: I80fe95ac6bb0e2d1ad6d993154ce45d492d941be
> Reviewed-on: https://chromium-review.googlesource.com/866855
> Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
> Reviewed-by: Hannes Payer <hpayer@chromium.org>
> Reviewed-by: Bill Budge <bbudge@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#50770}

TBR=bbudge@chromium.org,mstarzinger@chromium.org,hpayer@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: v8:6792
Change-Id: If4a205497ac83084a4092560363affb13b391462
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/883461Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Reviewed-by: 's avatarHannes Payer <hpayer@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50834}
parent 92d8e450
...@@ -200,11 +200,13 @@ class PageAllocator { ...@@ -200,11 +200,13 @@ class PageAllocator {
virtual void* GetRandomMmapAddr() = 0; virtual void* GetRandomMmapAddr() = 0;
/** /**
* Memory permissions. Note that V8 is W^X compliant. * Memory permissions.
*/ */
enum Permission { enum Permission {
kNoAccess, kNoAccess,
kReadWrite, kReadWrite,
// TODO(hpayer): Remove this flag. Memory should never be rwx.
kReadWriteExecute,
kReadExecute kReadExecute
}; };
......
...@@ -17,6 +17,8 @@ STATIC_ASSERT_ENUM(PageAllocator::kNoAccess, ...@@ -17,6 +17,8 @@ STATIC_ASSERT_ENUM(PageAllocator::kNoAccess,
base::OS::MemoryPermission::kNoAccess); base::OS::MemoryPermission::kNoAccess);
STATIC_ASSERT_ENUM(PageAllocator::kReadWrite, STATIC_ASSERT_ENUM(PageAllocator::kReadWrite,
base::OS::MemoryPermission::kReadWrite); base::OS::MemoryPermission::kReadWrite);
STATIC_ASSERT_ENUM(PageAllocator::kReadWriteExecute,
base::OS::MemoryPermission::kReadWriteExecute);
STATIC_ASSERT_ENUM(PageAllocator::kReadExecute, STATIC_ASSERT_ENUM(PageAllocator::kReadExecute,
base::OS::MemoryPermission::kReadExecute); base::OS::MemoryPermission::kReadExecute);
......
...@@ -36,6 +36,8 @@ DWORD GetProtectionFromMemoryPermission(OS::MemoryPermission access) { ...@@ -36,6 +36,8 @@ DWORD GetProtectionFromMemoryPermission(OS::MemoryPermission access) {
return PAGE_NOACCESS; return PAGE_NOACCESS;
case OS::MemoryPermission::kReadWrite: case OS::MemoryPermission::kReadWrite:
return PAGE_READWRITE; return PAGE_READWRITE;
case OS::MemoryPermission::kReadWriteExecute:
return PAGE_EXECUTE_READWRITE;
case OS::MemoryPermission::kReadExecute: case OS::MemoryPermission::kReadExecute:
return PAGE_EXECUTE_READ; return PAGE_EXECUTE_READ;
} }
......
...@@ -21,6 +21,9 @@ uint32_t GetProtectionFromMemoryPermission(OS::MemoryPermission access) { ...@@ -21,6 +21,9 @@ uint32_t GetProtectionFromMemoryPermission(OS::MemoryPermission access) {
return 0; // no permissions return 0; // no permissions
case OS::MemoryPermission::kReadWrite: case OS::MemoryPermission::kReadWrite:
return ZX_VM_FLAG_PERM_READ | ZX_VM_FLAG_PERM_WRITE; return ZX_VM_FLAG_PERM_READ | ZX_VM_FLAG_PERM_WRITE;
case OS::MemoryPermission::kReadWriteExecute:
return ZX_VM_FLAG_PERM_READ | ZX_VM_FLAG_PERM_WRITE |
ZX_VM_FLAG_PERM_EXECUTE;
case OS::MemoryPermission::kReadExecute: case OS::MemoryPermission::kReadExecute:
return ZX_VM_FLAG_PERM_READ | ZX_VM_FLAG_PERM_EXECUTE; return ZX_VM_FLAG_PERM_READ | ZX_VM_FLAG_PERM_EXECUTE;
} }
......
...@@ -109,6 +109,8 @@ int GetProtectionFromMemoryPermission(OS::MemoryPermission access) { ...@@ -109,6 +109,8 @@ int GetProtectionFromMemoryPermission(OS::MemoryPermission access) {
return PROT_NONE; return PROT_NONE;
case OS::MemoryPermission::kReadWrite: case OS::MemoryPermission::kReadWrite:
return PROT_READ | PROT_WRITE; return PROT_READ | PROT_WRITE;
case OS::MemoryPermission::kReadWriteExecute:
return PROT_READ | PROT_WRITE | PROT_EXEC;
case OS::MemoryPermission::kReadExecute: case OS::MemoryPermission::kReadExecute:
return PROT_READ | PROT_EXEC; return PROT_READ | PROT_EXEC;
} }
......
...@@ -747,6 +747,8 @@ DWORD GetProtectionFromMemoryPermission(OS::MemoryPermission access) { ...@@ -747,6 +747,8 @@ DWORD GetProtectionFromMemoryPermission(OS::MemoryPermission access) {
return PAGE_NOACCESS; return PAGE_NOACCESS;
case OS::MemoryPermission::kReadWrite: case OS::MemoryPermission::kReadWrite:
return PAGE_READWRITE; return PAGE_READWRITE;
case OS::MemoryPermission::kReadWriteExecute:
return PAGE_EXECUTE_READWRITE;
case OS::MemoryPermission::kReadExecute: case OS::MemoryPermission::kReadExecute:
return PAGE_EXECUTE_READ; return PAGE_EXECUTE_READ;
} }
......
...@@ -156,12 +156,12 @@ class V8_BASE_EXPORT OS { ...@@ -156,12 +156,12 @@ class V8_BASE_EXPORT OS {
static PRINTF_FORMAT(1, 0) void VPrintError(const char* format, va_list args); static PRINTF_FORMAT(1, 0) void VPrintError(const char* format, va_list args);
// Memory permissions. These should be kept in sync with the ones in // Memory permissions. These should be kept in sync with the ones in
// v8::PageAllocator. Note that there is on purpose no combination of // v8::PageAllocator.
// the "write" and "execute" permission, because V8 is W^X compliant.
// Avoid introducing such a combination as embedders might rely on it.
enum class MemoryPermission { enum class MemoryPermission {
kNoAccess, kNoAccess,
kReadWrite, kReadWrite,
// TODO(hpayer): Remove this flag. Memory should never be rwx.
kReadWriteExecute,
kReadExecute kReadExecute
}; };
......
...@@ -573,8 +573,8 @@ static inline uint8_t* AllocateAssemblerBuffer( ...@@ -573,8 +573,8 @@ static inline uint8_t* AllocateAssemblerBuffer(
size_t requested = v8::internal::AssemblerBase::kMinimalBufferSize) { size_t requested = v8::internal::AssemblerBase::kMinimalBufferSize) {
size_t page_size = v8::internal::AllocatePageSize(); size_t page_size = v8::internal::AllocatePageSize();
size_t alloc_size = RoundUp(requested, page_size); size_t alloc_size = RoundUp(requested, page_size);
void* result = v8::internal::AllocatePages(nullptr, alloc_size, page_size, void* result = v8::internal::AllocatePages(
v8::PageAllocator::kReadWrite); nullptr, alloc_size, page_size, v8::PageAllocator::kReadWriteExecute);
CHECK(result); CHECK(result);
*allocated = alloc_size; *allocated = alloc_size;
return static_cast<uint8_t*>(result); return static_cast<uint8_t*>(result);
......
...@@ -111,7 +111,7 @@ sigjmp_buf MemoryAllocationPermissionsTest::continuation_; ...@@ -111,7 +111,7 @@ sigjmp_buf MemoryAllocationPermissionsTest::continuation_;
TEST_F(MemoryAllocationPermissionsTest, DoTest) { TEST_F(MemoryAllocationPermissionsTest, DoTest) {
TestPermissions(PageAllocator::Permission::kNoAccess, false, false); TestPermissions(PageAllocator::Permission::kNoAccess, false, false);
TestPermissions(PageAllocator::Permission::kReadWrite, true, true); TestPermissions(PageAllocator::Permission::kReadWrite, true, true);
TestPermissions(PageAllocator::Permission::kReadExecute, true, false); TestPermissions(PageAllocator::Permission::kReadWriteExecute, true, true);
} }
#endif // V8_OS_POSIX #endif // V8_OS_POSIX
......
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