Skip to content

Commit

Permalink
[MERGE #3263 @litian2025] Fix Segmentation fault on Linux Skylake server
Browse files Browse the repository at this point in the history
Merge pull request #3263 from litian2025:page_execute

The root cause of the issue is because on Windows, if memory is protected as  PAGE_EXECUTE, it is implies that it is also PAGE_EXECUTE_READ. This works fine on Haswell server and Skylake client on Linux. However, On Linux Skylake server, it is more restrict and explict, PROT_EXE is Executable only, not readable.  The fix is to ensure PAL emulates Windows API. This fix the issue #3262.
  • Loading branch information
obastemur committed Jul 11, 2017
2 parents 19057a6 + 31b1d8f commit d3a96f4
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 27 deletions.
4 changes: 2 additions & 2 deletions lib/Backend/EmitBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ EmitBufferManager<TAlloc, TPreReservedAlloc, SyncObject>::AllocateBuffer(__in si
#if DBG
MEMORY_BASIC_INFORMATION memBasicInfo;
size_t resultBytes = VirtualQueryEx(this->processHandle, allocation->allocation->address, &memBasicInfo, sizeof(memBasicInfo));
Assert(resultBytes == 0 || memBasicInfo.Protect == PAGE_EXECUTE);
Assert(resultBytes == 0 || memBasicInfo.Protect == PAGE_EXECUTE_READ);
#endif

return allocation;
Expand Down Expand Up @@ -334,7 +334,7 @@ bool EmitBufferManager<TAlloc, TPreReservedAlloc, SyncObject>::IsBufferExecuteRe
AutoRealOrFakeCriticalSection<SyncObject> autoCs(&this->criticalSection);
MEMORY_BASIC_INFORMATION memBasicInfo;
size_t resultBytes = VirtualQuery(allocation->allocation->address, &memBasicInfo, sizeof(memBasicInfo));
return resultBytes != 0 && memBasicInfo.Protect == PAGE_EXECUTE;
return resultBytes != 0 && memBasicInfo.Protect == PAGE_EXECUTE_READ;
}
#endif

Expand Down
8 changes: 4 additions & 4 deletions lib/Backend/JITThunkEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ JITThunkEmitter<TAlloc>::CreateThunk(uintptr_t entryPoint)

if (IsThunkPageEmpty(pageStartAddress))
{
if (this->codeAllocator->Alloc((PVOID)pageStartAddress, AutoSystemInfo::PageSize, MEM_COMMIT, PAGE_EXECUTE, true) == nullptr)
if (this->codeAllocator->Alloc((PVOID)pageStartAddress, AutoSystemInfo::PageSize, MEM_COMMIT, PAGE_EXECUTE_READ, true) == nullptr)
{
this->codeAllocator->FreeLocal(localPageAddress);
return NULL;
Expand Down Expand Up @@ -165,7 +165,7 @@ JITThunkEmitter<TAlloc>::EnsureInitialized()
// check again because we did the first one outside of lock
if (this->baseAddress == NULL)
{
this->baseAddress = (uintptr_t)this->codeAllocator->Alloc(nullptr, TotalThunkSize, MEM_RESERVE, PAGE_EXECUTE, true);
this->baseAddress = (uintptr_t)this->codeAllocator->Alloc(nullptr, TotalThunkSize, MEM_RESERVE, PAGE_EXECUTE_READ, true);
}
}
return this->baseAddress;
Expand Down Expand Up @@ -230,7 +230,7 @@ JITThunkEmitter<VirtualAllocWrapper>::ProtectPage(void * address)
AutoEnableDynamicCodeGen enableCodeGen(true);
#endif
DWORD oldProtect;
BOOL result = VirtualProtectEx(this->processHandle, address, ThunkSize, PAGE_EXECUTE, &oldProtect);
BOOL result = VirtualProtectEx(this->processHandle, address, ThunkSize, PAGE_EXECUTE_READ, &oldProtect);
AssertOrFailFast(result && oldProtect == PAGE_EXECUTE_READWRITE);
}

Expand All @@ -243,7 +243,7 @@ JITThunkEmitter<VirtualAllocWrapper>::UnprotectPage(void * address)
#endif
DWORD oldProtect;
BOOL result = VirtualProtectEx(this->processHandle, address, ThunkSize, PAGE_EXECUTE_READWRITE, &oldProtect);
AssertOrFailFast(result && oldProtect == PAGE_EXECUTE);
AssertOrFailFast(result && oldProtect == PAGE_EXECUTE_READ);
}

#if ENABLE_OOP_NATIVE_CODEGEN
Expand Down
4 changes: 2 additions & 2 deletions lib/Common/CommonDefines.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@

// Memory Protections
#ifdef _CONTROL_FLOW_GUARD
#define PAGE_EXECUTE_RO_TARGETS_INVALID (PAGE_EXECUTE | PAGE_TARGETS_INVALID)
#define PAGE_EXECUTE_RO_TARGETS_INVALID (PAGE_EXECUTE_READ | PAGE_TARGETS_INVALID)
#else
#define PAGE_EXECUTE_RO_TARGETS_INVALID (PAGE_EXECUTE)
#define PAGE_EXECUTE_RO_TARGETS_INVALID (PAGE_EXECUTE_READ)
#endif

//----------------------------------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions lib/Common/Memory/CommonMemoryPch.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ typedef _Return_type_success_(return >= 0) LONG NTSTATUS;
#ifdef _CONTROL_FLOW_GUARD
#define PAGE_EXECUTE_RW_TARGETS_INVALID (PAGE_EXECUTE_READWRITE | PAGE_TARGETS_INVALID)
#define PAGE_EXECUTE_RW_TARGETS_NO_UPDATE (PAGE_EXECUTE_READWRITE | PAGE_TARGETS_NO_UPDATE)
#define PAGE_EXECUTE_RO_TARGETS_NO_UPDATE (PAGE_EXECUTE | PAGE_TARGETS_NO_UPDATE)
#define PAGE_EXECUTE_RO_TARGETS_NO_UPDATE (PAGE_EXECUTE_READ | PAGE_TARGETS_NO_UPDATE)
#else
#define PAGE_EXECUTE_RW_TARGETS_INVALID (PAGE_EXECUTE_READWRITE)
#define PAGE_EXECUTE_RW_TARGETS_NO_UPDATE (PAGE_EXECUTE_READWRITE)
#define PAGE_EXECUTE_RO_TARGETS_NO_UPDATE (PAGE_EXECUTE)
#define PAGE_EXECUTE_RO_TARGETS_NO_UPDATE (PAGE_EXECUTE_READ)
#endif
16 changes: 8 additions & 8 deletions lib/Common/Memory/CustomHeap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ Allocation* Heap<TAlloc, TPreReservedAlloc>::Alloc(size_t bytes, ushort pdataCou
}
else
{
Assert(memBasicInfo.Protect == PAGE_EXECUTE);
Assert(memBasicInfo.Protect == PAGE_EXECUTE_READ);
}
#endif

Expand All @@ -291,7 +291,7 @@ BOOL Heap<TAlloc, TPreReservedAlloc>::ProtectAllocationWithExecuteReadWrite(Allo
{
protectFlags = PAGE_EXECUTE_READWRITE;
}
return this->ProtectAllocation(allocation, protectFlags, PAGE_EXECUTE, addressInPage);
return this->ProtectAllocation(allocation, protectFlags, PAGE_EXECUTE_READ, addressInPage);
}

template<typename TAlloc, typename TPreReservedAlloc>
Expand All @@ -304,7 +304,7 @@ BOOL Heap<TAlloc, TPreReservedAlloc>::ProtectAllocationWithExecuteReadOnly(Alloc
}
else
{
protectFlags = PAGE_EXECUTE;
protectFlags = PAGE_EXECUTE_READ;
}
return this->ProtectAllocation(allocation, protectFlags, PAGE_EXECUTE_READWRITE, addressInPage);
}
Expand Down Expand Up @@ -418,7 +418,7 @@ Allocation* Heap<TAlloc, TPreReservedAlloc>::AllocLargeObject(size_t bytes, usho
}
else
{
protectFlags = PAGE_EXECUTE;
protectFlags = PAGE_EXECUTE_READ;
}
this->codePageAllocators->ProtectPages(address, pages, segment, protectFlags /*dwVirtualProtectFlags*/, PAGE_READWRITE /*desiredOldProtectFlags*/);
}
Expand All @@ -443,7 +443,7 @@ Allocation* Heap<TAlloc, TPreReservedAlloc>::AllocLargeObject(size_t bytes, usho
}
else
{
Assert(memBasicInfo.Protect == PAGE_EXECUTE);
Assert(memBasicInfo.Protect == PAGE_EXECUTE_READ);
}
#endif

Expand Down Expand Up @@ -677,7 +677,7 @@ Page* Heap<TAlloc, TPreReservedAlloc>::AllocNewPage(BucketId bucket, bool canAll
}
else
{
protectFlags = PAGE_EXECUTE;
protectFlags = PAGE_EXECUTE_READ;
}

//Change the protection of the page to Read-Only Execute, before adding it to the bucket list.
Expand Down Expand Up @@ -867,7 +867,7 @@ bool Heap<TAlloc, TPreReservedAlloc>::FreeAllocation(Allocation* object)
EnsureAllocationExecuteWriteable(object);
FreeAllocationHelper(object, index, length);

// after freeing part of the page, the page should be in PAGE_EXECUTE_READWRITE protection, and turning to PAGE_EXECUTE (always with TARGETS_NO_UPDATE state)
// after freeing part of the page, the page should be in PAGE_EXECUTE_READWRITE protection, and turning to PAGE_EXECUTE_READ (always with TARGETS_NO_UPDATE state)

DWORD protectFlags = 0;

Expand All @@ -877,7 +877,7 @@ bool Heap<TAlloc, TPreReservedAlloc>::FreeAllocation(Allocation* object)
}
else
{
protectFlags = PAGE_EXECUTE;
protectFlags = PAGE_EXECUTE_READ;
}

this->codePageAllocators->ProtectPages(page->address, 1, segment, protectFlags, PAGE_EXECUTE_READWRITE);
Expand Down
8 changes: 4 additions & 4 deletions lib/Common/Memory/CustomHeap.h
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,8 @@ class Heap
DWORD EnsurePageReadWrite(Page* page)
{
Assert(!page->isDecommitted);
this->codePageAllocators->ProtectPages(page->address, 1, page->segment, readWriteFlags, PAGE_EXECUTE);
return PAGE_EXECUTE;
this->codePageAllocators->ProtectPages(page->address, 1, page->segment, readWriteFlags, PAGE_EXECUTE_READ);
return PAGE_EXECUTE_READ;
}

template<DWORD readWriteFlags>
Expand All @@ -491,8 +491,8 @@ class Heap
{
if (allocation->IsLargeAllocation())
{
this->ProtectAllocation(allocation, readWriteFlags, PAGE_EXECUTE);
return PAGE_EXECUTE;
this->ProtectAllocation(allocation, readWriteFlags, PAGE_EXECUTE_READ);
return PAGE_EXECUTE_READ;
}
else
{
Expand Down
8 changes: 4 additions & 4 deletions lib/Common/Memory/SectionAllocWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ PVOID MapView(HANDLE process, HANDLE sectionHandle, size_t size, size_t offset,
{
return nullptr;
}
flags = AutoSystemInfo::Data.IsCFGEnabled() ? PAGE_EXECUTE_RO_TARGETS_INVALID : PAGE_EXECUTE;
flags = AutoSystemInfo::Data.IsCFGEnabled() ? PAGE_EXECUTE_RO_TARGETS_INVALID : PAGE_EXECUTE_READ;
}

#if USEFILEMAP2
Expand Down Expand Up @@ -590,7 +590,7 @@ SectionAllocWrapper::Alloc(LPVOID requestAddress, size_t dwSize, DWORD allocatio

if ((allocationType & MEM_COMMIT) == MEM_COMMIT)
{
const DWORD allocProtectFlags = AutoSystemInfo::Data.IsCFGEnabled() ? PAGE_EXECUTE_RO_TARGETS_INVALID : PAGE_EXECUTE;
const DWORD allocProtectFlags = AutoSystemInfo::Data.IsCFGEnabled() ? PAGE_EXECUTE_RO_TARGETS_INVALID : PAGE_EXECUTE_READ;
address = VirtualAllocEx(this->process, address, dwSize, MEM_COMMIT, allocProtectFlags);
}
}
Expand Down Expand Up @@ -717,7 +717,7 @@ PreReservedSectionAllocWrapper::IsInRange(void * address)
{
MEMORY_BASIC_INFORMATION memBasicInfo;
size_t bytes = VirtualQueryEx(this->process, address, &memBasicInfo, sizeof(memBasicInfo));
Assert(bytes == 0 || (memBasicInfo.State == MEM_COMMIT && memBasicInfo.AllocationProtect == PAGE_EXECUTE));
Assert(bytes == 0 || (memBasicInfo.State == MEM_COMMIT && memBasicInfo.AllocationProtect == PAGE_EXECUTE_READ));
}
#endif
return isInRange;
Expand Down Expand Up @@ -920,7 +920,7 @@ LPVOID PreReservedSectionAllocWrapper::Alloc(LPVOID lpAddress, size_t dwSize, DW
AutoEnableDynamicCodeGen enableCodeGen;
#endif

const DWORD allocProtectFlags = AutoSystemInfo::Data.IsCFGEnabled() ? PAGE_EXECUTE_RO_TARGETS_INVALID : PAGE_EXECUTE;
const DWORD allocProtectFlags = AutoSystemInfo::Data.IsCFGEnabled() ? PAGE_EXECUTE_RO_TARGETS_INVALID : PAGE_EXECUTE_READ;
allocatedAddress = (char *)VirtualAllocEx(this->process, addressToReserve, dwSize, MEM_COMMIT, allocProtectFlags);
if (allocatedAddress == nullptr)
{
Expand Down
2 changes: 1 addition & 1 deletion pal/src/map/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ INT W32toUnixAccessControl( IN DWORD flProtect )
MemAccessControl = PROT_EXEC | PROT_READ | PROT_WRITE;
break;
case PAGE_EXECUTE :
MemAccessControl = PROT_EXEC;
MemAccessControl = PROT_EXEC | PROT_READ; // WinAPI PAGE_EXECUTE also implies readable
break;
case PAGE_EXECUTE_READ :
MemAccessControl = PROT_EXEC | PROT_READ;
Expand Down

0 comments on commit d3a96f4

Please sign in to comment.