Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable W^X in Rosetta emulated x64 containers on macOS #102509

Merged
merged 4 commits into from
May 23, 2024
Merged

Disable W^X in Rosetta emulated x64 containers on macOS #102509

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
42e81bc
Disable W^X on x64 in rosetta based container
janvorli May 20, 2024
c737b9a
Rework based on PR feedback
janvorli May 23, 2024
7c734b0
Check only for Rosetta
janvorli May 23, 2024
a6833c0
Enable the rosetta check for x86 too
janvorli May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions src/coreclr/minipal/Unix/doublemapping.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,90 @@ static const off_t MaxDoubleMappedSize = UINT_MAX;

#endif // TARGET_OSX

#if defined(TARGET_AMD64) && !defined(TARGET_OSX)
Copy link
Member

@am11 am11 May 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any technical issue that require us to restrict this run-time introspection for these platforms? If we don't restrict it, then it will cover qemu cases for other archs as well (e.g. #97729 (comment)), and prevent user from manually setting DOTNET_EnableWriteXorExecute=0 to run .NET applications.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am actually not convinced whether such detection is required? Perhaps we can just document that the config needs to be disabled for emulated cases?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember once @jkotas mentioned that we wanted to support rosetta2 on macOS as a first-class platform, and therefore we are handling it explicitly with sysctl.proc_translated in a few places (coreclr, nativelibs and shell scripts and even in official build infra for downloading x64 on arm64 in translated process). I think it might be better to bring this to IsProcessTranslated plan as well without the run-time detection?

runtime/src/coreclr/minipal/Unix/doublemapping.cpp

Line 28 in 362a95d

bool IsProcessTranslated()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sysctl.proc_translated is not going to help. This is about running Linux docker containers. We do not know about sysctl.proc_translated equivalent for Linux docker containers running under Rosetta.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see, it makes sense. Could we enable this feature detection for other platforms as well (which will cover qemu scenarios)? (assuming the overhead due to this one-time check is negligible)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@am11 I've tried to look at the discussion you have linked, but I have not seen any evidence of the culprit being inability to double map. But I might have missed that. Can you please point me to such information? The point is, if it is not a problem of double mapping, this check may not be able to check the other issue.
When I started to investigate this issue, I actually still had a docker desktop that used QEMU for x64 emulation on arm64 and even with W^X disabled, attempt to build .NET runtime was failing at some point due to SIGSEGV. Only after that I've realized I need to upgrade docker to get the version that used Rosetta to be able to make it work even with W^X disabled.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@janvorli, I was running into qemu: uncaught target signal 11 (Segmentation fault) which was fixed with DOTNET_EnableWriteXorExecute=0. Later dotnet publish was failing a runtime assertion on qemu arm target. But to get to that late assertion sigsegv was the roadblock that DOTNET_EnableWriteXorExecute=0 fixed. In the context of this PR, I thought it might be useful to keep the condition a bit relaxed.


extern "C" int VerifyDoubleMapping1();
extern "C" void VerifyDoubleMapping1_End();
extern "C" int VerifyDoubleMapping2();
extern "C" void VerifyDoubleMapping2_End();

// Verify that the double mapping works correctly, including cases when the executable code page is modified after
// the code is executed.
bool VerifyDoubleMapping(int fd)
{
bool result = false;
void *mapperHandle = (void*)(size_t)fd;
void *pCommittedPage = NULL;
void *pWriteablePage = NULL;
int testCallResult;

typedef int (*VerificationFunctionPtr)();
VerificationFunctionPtr pVerificationFunction;

size_t pageSize = getpagesize();

void *pExecutablePage = VMToOSInterface::ReserveDoubleMappedMemory(mapperHandle, 0, pageSize, NULL, NULL);

if (pExecutablePage == NULL)
{
goto Cleanup;
}

pCommittedPage = VMToOSInterface::CommitDoubleMappedMemory(pExecutablePage, pageSize, true);
if (pCommittedPage == NULL)
{
goto Cleanup;
}

pWriteablePage = VMToOSInterface::GetRWMapping(mapperHandle, pCommittedPage, 0, pageSize);
if (pWriteablePage == NULL)
{
goto Cleanup;
}

// First copy a method of a simple function that returns 1 into the writeable mapping
memcpy(pWriteablePage, (void*)VerifyDoubleMapping1, (char*)VerifyDoubleMapping1_End - (char*)VerifyDoubleMapping1);
pVerificationFunction = (VerificationFunctionPtr)pExecutablePage;
// Invoke the function via the executable mapping. It should return 1.
testCallResult = pVerificationFunction();
if (testCallResult != 1)
{
goto Cleanup;
}

VMToOSInterface::ReleaseRWMapping(pWriteablePage, pageSize);
pWriteablePage = VMToOSInterface::GetRWMapping(mapperHandle, pCommittedPage, 0, pageSize);
if (pWriteablePage == NULL)
{
goto Cleanup;
}

// Now overwrite the first function by a second one that returns 2 using the writeable mapping
memcpy(pWriteablePage, (void*)VerifyDoubleMapping2, (char*)VerifyDoubleMapping2_End - (char*)VerifyDoubleMapping2);
pVerificationFunction = (VerificationFunctionPtr)pExecutablePage;
testCallResult = pVerificationFunction();
// Invoke the function via the executable mapping again. It should return 2 now.
// This doesn't work when running x64 code in docker on macOS Arm64 where the code is not re-translated by Rosetta
Copy link
Member

@jkotas jkotas May 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering whether it is possible to make it work by issuing flush instruction cache syscall. Is there a syscall like that on Linux x64?

Similar to what we do for Windows:

runtime/src/coreclr/vm/amd64/cgencpu.h

Lines 588 to 605 in 362a95d

// ClrFlushInstructionCache is used when we want to call FlushInstructionCache
// for a specific architecture in the common code, but not for other architectures.
// We call ClrFlushInstructionCache whenever we create or modify code in the heap.
// Currently ClrFlushInstructionCache has no effect on AMD64
//
inline BOOL ClrFlushInstructionCache(LPCVOID pCodeAddr, size_t sizeOfCode, bool hasCodeExecutedBefore = false)
{
if (hasCodeExecutedBefore)
{
FlushInstructionCache(GetCurrentProcess(), pCodeAddr, sizeOfCode);
}
else
{
MemoryBarrier();
}
return TRUE;
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apple folks told me in the past that Rosetta doesn't support this scenario, but I can add the cache flushing. In fact, it should be in this testing method anyways.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be specific, the problem is that Rosetta JITs the x64 code into arm64 on first execution and uses the result ever after. Since it doesn't get any notification on the executable code modification via a secondary mapping, it doesn't re-JIT it and keeps using the old code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for the cache flushing, there is the __builtin___clear_cache, but it doesn't generate any code on x64. And there is no syscall to do that on Linux.

if (testCallResult == 2)
{
result = true;
}

Cleanup:
if (pWriteablePage != NULL)
{
VMToOSInterface::ReleaseRWMapping(pWriteablePage, pageSize);
}

if (pExecutablePage != NULL)
{
VMToOSInterface::ReleaseDoubleMappedMemory(mapperHandle, pExecutablePage, 0, pageSize);
}

return result;
}
#endif // TARGET_AMD64 && !TARGET_OSX

bool VMToOSInterface::CreateDoubleMemoryMapper(void** pHandle, size_t *pMaxExecutableCodeSize)
{
#ifndef TARGET_OSX
Expand Down Expand Up @@ -74,6 +158,14 @@ bool VMToOSInterface::CreateDoubleMemoryMapper(void** pHandle, size_t *pMaxExecu
return false;
}

#if defined(TARGET_AMD64) && !defined(TARGET_OSX)
if (!VerifyDoubleMapping(fd))
{
close(fd);
return false;
Copy link
Member

@jkotas jkotas May 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this going to result in W^X being disabled?

If I am reading the code correctly, it is going to result into startup failure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is going to result in W^X being disabled - I've tested it in a x64 docker container on my M1. See the caller of the VMToOSInterface::CreateDoubleMemoryMapper:

runtime/src/coreclr/utilcode/executableallocator.cpp

Lines 282 to 298 in 7452305

bool ExecutableAllocator::Initialize()
{
LIMITED_METHOD_CONTRACT;
if (IsDoubleMappingEnabled())
{
if (!VMToOSInterface::CreateDoubleMemoryMapper(&m_doubleMemoryMapperHandle, &m_maxExecutableCodeSize))
{
g_isWXorXEnabled = false;
return true;
}
m_CriticalSection = ClrCreateCriticalSection(CrstExecutableAllocatorLock,CrstFlags(CRST_UNSAFE_ANYMODE | CRST_DEBUGGER_THREAD));
}
return true;
}

}
#endif // TARGET_AMD64 && !TARGET_OSX

*pMaxExecutableCodeSize = MaxDoubleMappedSize;
*pHandle = (void*)(size_t)fd;
#else // !TARGET_OSX
Expand Down
11 changes: 11 additions & 0 deletions src/coreclr/vm/amd64/asmhelpers.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,3 +311,14 @@ LEAF_ENTRY GetTlsIndexObjectDescOffset, _TEXT
int 3
LEAF_END GetTlsIndexObjectDescOffset, _TEXT
#endif

# These functions are used to verify that double mapping used to implement W^X works.
jkotas marked this conversation as resolved.
Show resolved Hide resolved
LEAF_ENTRY VerifyDoubleMapping1, _TEXT
mov rax, 1
ret
LEAF_END_MARKED VerifyDoubleMapping1, _TEXT

LEAF_ENTRY VerifyDoubleMapping2, _TEXT
mov rax, 2
ret
LEAF_END_MARKED VerifyDoubleMapping2, _TEXT