Skip to content

Commit

Permalink
Clean up Signals and remove hardened fallback
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=271766
rdar://125256111

Reviewed by Justin Michaud.

This change does a couple of things:

1) Refactor the signal handler API after https://commits.webkit.org/276579@main which had mach exception users
   pass the mask of exceptions they would ever want to handle at initialization time (before they register their handler).
   Instead the new signal handler API registers our handlers with the kernel at finalization time. At which point we
   have a list of every exception handler added.

2) Remove the finalizeSignalHandlers function and finalize signal handlers when WTF::Config finalizes.

3) Remove disableSignalHandling function as it never really worked (signals could still get registered before we called disable)
   and we mostly rely on the sandbox to block signals in lockdown mode.

3) Rename SignalHandlers::InitState entries to better reflect the new design and use it for all configurations not just mach ports.

4) Remove the fallback we added to https://commits.webkit.org/276579@main as a stopgap for internal folks running on old sandboxes.

5) Remove `isX86BinaryRunningOnARM()` check since that didn't seem to work anyway. We'll probably have to remove signal handling under
   Rosetta as I couldn't get it to work.

6) Save the secret key for our hardened handler in Signals.cpp before finalization and only let API users see it via presignReturnPCForHandler
   so they can't accidentally save it somewhere an attacker could see later. We also carefully zero it at finalization time after passing
   it to the kernel via `task_register_hardened_exception_handler`.

7) Move disabledFreezingForTesting to WTF::Config since that's the config that actually does the freezing not JSC::Config.
   This allows WTF::Config to control finalizing signal handlers.

8) Remove some cases from com.apple.WebKit.WebContent.sb.in to simplify the profile slightly. We don't need to guard the
   `(require-not (webcontent-process-launched))` check on `ENABLE(BLOCK_SET_EXCEPTION_PORTS)` because we have
   `webcontent-process-launched` anywhere we `HAVE(HARDENED_MACH_EXCEPTIONS)` anyway.

* Source/JavaScriptCore/jsc.cpp:
(main):
(CommandLine::parseArguments):
* Source/JavaScriptCore/runtime/InitializeThreading.cpp:
(JSC::initialize):
* Source/JavaScriptCore/runtime/JSCConfig.cpp:
(JSC::Config::disableFreezingForTesting): Deleted.
* Source/JavaScriptCore/runtime/JSCConfig.h:
(JSC::Config::disableFreezingForTesting):
(JSC::Config::finalize):
(JSC::Config::permanentlyFreeze): Deleted.
* Source/JavaScriptCore/runtime/VM.cpp:
(JSC::VM::VM):
* Source/JavaScriptCore/runtime/VMEntryScope.cpp:
(JSC::VMEntryScope::setUpSlow):
* Source/JavaScriptCore/runtime/VMTraps.cpp:
* Source/JavaScriptCore/tools/JSDollarVM.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION_WITH_ATTRIBUTES):
* Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp:
(JSC::Wasm::trapHandler):
(JSC::Wasm::activateSignalingMemory):
(JSC::Wasm::prepareSignalingMemory):
(JSC::Wasm::MachExceptionSigningKey::MachExceptionSigningKey): Deleted.
* Source/JavaScriptCore/wasm/WasmFaultSignalHandler.h:
* Source/WTF/wtf/PlatformRegisters.cpp:
(WTF::threadStatePCInternal):
* Source/WTF/wtf/Threading.cpp:
(WTF::initialize):
* Source/WTF/wtf/WTFConfig.cpp:
(WTF::Config::initialize):
(WTF::Config::finalize):
(WTF::Config::permanentlyFreeze):
(WTF::Config::disableFreezingForTesting):
* Source/WTF/wtf/WTFConfig.h:
* Source/WTF/wtf/threads/Signals.cpp:
(WTF::SignalHandlers::add):
(WTF::SignalHandlers::presignReturnPCForHandler):
(WTF::initMachExceptionHandlerThread):
(WTF::toMachMask):
(WTF::setExceptionPorts):
(WTF::activeThreads):
(WTF::registerThreadForMachExceptionHandling):
(WTF::activateSignalHandlersFor):
(WTF::addSignalHandler):
(WTF::disableSignalHandling):
(WTF::jscSignalHandler):
(WTF::SignalHandlers::initialize):
(WTF::SignalHandlers::finalize):
(WTF::finalizeSignalHandlers): Deleted.
* Source/WTF/wtf/threads/Signals.h:
(WTF::toMachMask): Deleted.
(WTF::initializeSignalHandling): Deleted.
(WTF::disableSignalHandling): Deleted.
* Source/WTF/wtf/win/SignalsWin.cpp:
(WTF::SignalHandlers::add):
(WTF::addSignalHandler):
(WTF::activateSignalHandlersFor):
(WTF::SignalHandlers::initialize):
(WTF::SignalHandlers::finalize):
(WTF::finalizeSignalHandlers): Deleted.
* Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb.in:

Canonical link: https://commits.webkit.org/277136@main
  • Loading branch information
kmiller68 committed Apr 5, 2024
1 parent 0713e3e commit b3eceeb
Show file tree
Hide file tree
Showing 20 changed files with 232 additions and 320 deletions.
18 changes: 0 additions & 18 deletions Source/JavaScriptCore/jsc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3522,9 +3522,6 @@ int main(int argc, char** argv WTF_TZONE_EXTRA_MAIN_ARGS)

#if OS(UNIX)
if (getenv("JS_SHELL_WAIT_FOR_SIGUSR2_TO_EXIT")) {
uint32_t key = 0;
int mask = 0;
initializeSignalHandling(key, mask);
addSignalHandler(Signal::Usr, SignalHandler([&] (Signal, SigInfo&, PlatformRegisters&) {
dataLogLn("Signal handler hit, we can exit now.");
waitToExit.signal();
Expand Down Expand Up @@ -3998,20 +3995,6 @@ void CommandLine::parseArguments(int argc, char** argv)
}
if (!strcmp(arg, "-s")) {
#if OS(UNIX)
uint32_t key = 0;
int mask = 0;
#if HAVE(MACH_EXCEPTIONS)
mask |= toMachMask(Signal::IllegalInstruction);
mask |= toMachMask(Signal::AccessFault);
mask |= toMachMask(Signal::FloatingPoint);
mask |= toMachMask(Signal::Breakpoint);
#if !OS(DARWIN)
mask |= toMachMask(Signal::Abort);
#endif // !OS(DARWIN)
#endif // HAVE(MACH_EXCEPTIONS)

initializeSignalHandling(key, mask);

SignalAction (*exit)(Signal, SigInfo&, PlatformRegisters&) = [] (Signal, SigInfo&, PlatformRegisters&) {
dataLogLn("Signal handler hit. Exiting with status 0");
// Deliberate exit with a SIGKILL code greater than 130.
Expand All @@ -4034,7 +4017,6 @@ void CommandLine::parseArguments(int argc, char** argv)
addSignalHandler(Signal::Abort, SignalHandler(exit));
activateSignalHandlersFor(Signal::Abort);
#endif
finalizeSignalHandlers();
#endif
continue;
}
Expand Down
14 changes: 1 addition & 13 deletions Source/JavaScriptCore/runtime/InitializeThreading.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,23 +136,11 @@ void initialize()
WTF::fastEnableMiniMode();

if (Wasm::isSupported() || !Options::usePollingTraps()) {
// JSLock::lock() can call registerThreadForMachExceptionHandling() which crashes if this has not been called first.
int mask = 0;
#if CPU(ARM64E) && HAVE(HARDENED_MACH_EXCEPTIONS)
JSC::Wasm::MachExceptionSigningKey keygen;
uint32_t signingKey = keygen.randomSigningKey;
mask |= toMachMask(Signal::AccessFault);
#else
uint32_t signingKey = 0;
#endif // CPU(ARM64E) && HAVE(HARDENED_MACH_EXCEPTIONS)
initializeSignalHandling(signingKey, mask);

if (!Options::usePollingTraps())
VMTraps::initializeSignals();
if (Wasm::isSupported())
Wasm::prepareSignalingMemory();
} else
disableSignalHandling();
}

WTF::compilerFence();
RELEASE_ASSERT(!g_jscConfig.initializeHasBeenCalled);
Expand Down
6 changes: 0 additions & 6 deletions Source/JavaScriptCore/runtime/JSCConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,6 @@ Config& Config::singleton()
return g_jscConfig;
}

void Config::disableFreezingForTesting()
{
RELEASE_ASSERT(!g_jscConfig.isPermanentlyFrozen());
g_jscConfig.disabledFreezingForTesting = true;
}

void Config::enableRestrictedOptions()
{
RELEASE_ASSERT(!g_jscConfig.isPermanentlyFrozen());
Expand Down
6 changes: 3 additions & 3 deletions Source/JavaScriptCore/runtime/JSCConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ using JITWriteSeparateHeapsFunction = void (*)(off_t, const void*, size_t);
struct Config {
static Config& singleton();

JS_EXPORT_PRIVATE static void disableFreezingForTesting();
static void disableFreezingForTesting() { g_wtfConfig.disableFreezingForTesting(); }
JS_EXPORT_PRIVATE static void enableRestrictedOptions();
static void permanentlyFreeze() { WTF::Config::permanentlyFreeze(); }
static void finalize() { WTF::Config::finalize(); }

static void configureForTesting()
{
Expand All @@ -60,8 +60,8 @@ struct Config {
// All the fields in this struct should be chosen such that their
// initial value is 0 / null / falsy because Config is instantiated
// as a global singleton.
// FIXME: We should use a placement new constructor from JSC::initialize so we can use default initializers.

bool disabledFreezingForTesting;
bool restrictedOptionsEnabled;
bool jitDisabled;
bool vmCreationDisallowed;
Expand Down
3 changes: 1 addition & 2 deletions Source/JavaScriptCore/runtime/VM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -434,8 +434,7 @@ VM::VM(VMType vmType, HeapType heapType, WTF::RunLoop* runLoop, bool* success)
jitSizeStatistics = makeUnique<JITSizeStatistics>();
#endif

if (!g_jscConfig.disabledFreezingForTesting)
Config::permanentlyFreeze();
Config::finalize();

// We must set this at the end only after the VM is fully initialized.
WTF::storeStoreFence();
Expand Down
3 changes: 1 addition & 2 deletions Source/JavaScriptCore/runtime/VMEntryScope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ void VMEntryScope::setUpSlow()
if (Wasm::isSupported())
Wasm::startTrackingCurrentThread();
#if HAVE(MACH_EXCEPTIONS)
if (g_wtfConfig.signalHandlers.initState == WTF::SignalHandlers::InitState::AddedHandlers)
registerThreadForMachExceptionHandling(thread);
registerThreadForMachExceptionHandling(thread);
#endif
}

Expand Down
1 change: 0 additions & 1 deletion Source/JavaScriptCore/runtime/VMTraps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ class VMTraps::SignalSender final : public AutomaticThread {
, m_vm(vm)
{
activateSignalHandlersFor(Signal::AccessFault);
finalizeSignalHandlers();
}

static void initializeSignals()
Expand Down
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/tools/JSDollarVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2922,7 +2922,7 @@ JSC_DEFINE_HOST_FUNCTION_WITH_ATTRIBUTES(functionCallWithStackSize, SUPPRESS_ASA
return throwVMError(globalObject, throwScope, "Not supported for this platform"_s);

#if ENABLE(ASSEMBLER)
if (g_jscConfig.isPermanentlyFrozen() || !g_jscConfig.disabledFreezingForTesting)
if (g_jscConfig.isPermanentlyFrozen() || !g_wtfConfig.disabledFreezingForTesting)
return throwVMError(globalObject, throwScope, "Options are frozen"_s);

if (callFrame->argumentCount() < 2)
Expand Down
27 changes: 8 additions & 19 deletions Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
#include "WasmMemory.h"
#include "WasmThunks.h"
#include <wtf/CodePtr.h>
#include <wtf/CryptographicallyRandomNumber.h>
#include <wtf/HashSet.h>
#include <wtf/Lock.h>
#include <wtf/threads/Signals.h>
Expand All @@ -49,18 +48,8 @@ namespace JSC { namespace Wasm {
using WTF::CodePtr;

#if CPU(ARM64E) && HAVE(HARDENED_MACH_EXCEPTIONS)
void* presignedTrampoline = { };

MachExceptionSigningKey::MachExceptionSigningKey()
{
// Sign the trampoline pointer using a random diversifier and stash it away before webcontent has started so that
// even a PAC signing gadget cannot fake this random diversifier
randomSigningKey = WTF::cryptographicallyRandomNumber<uint32_t>() & __DARWIN_ARM_THREAD_STATE64_USER_DIVERSIFIER_MASK;
uint64_t diversifier = ptrauth_blend_discriminator((void *)(unsigned long)randomSigningKey, ptrauth_string_discriminator("pc"));
presignedTrampoline = JSC::LLInt::getCodePtr<CFunctionPtrTag>(wasm_throw_from_fault_handler_trampoline_reg_instance).untaggedPtr();
presignedTrampoline = ptrauth_sign_unauthenticated(presignedTrampoline, ptrauth_key_function_pointer, diversifier);
}
#endif // CPU(ARM64E) && HAVE(HARDENED_MACH_EXCEPTIONS)
void* presignedTrampoline { nullptr };
#endif

namespace {
namespace WasmFaultSignalHandlerInternal {
Expand Down Expand Up @@ -115,12 +104,10 @@ static SignalAction trapHandler(Signal signal, SigInfo& sigInfo, PlatformRegiste

if (didFaultInWasm(faultingInstruction)) {
#if CPU(ARM64E) && HAVE(HARDENED_MACH_EXCEPTIONS)
if (!WTF::fallbackToOldExceptions.load()) {
MachineContext::setInstructionPointer(context, presignedTrampoline);
return SignalAction::Handled;
}
#endif
MachineContext::setInstructionPointer(context, presignedTrampoline);
#else
MachineContext::setInstructionPointer(context, LLInt::getCodePtr<CFunctionPtrTag>(wasm_throw_from_fault_handler_trampoline_reg_instance));
#endif
return SignalAction::Handled;
}
}
Expand All @@ -139,7 +126,6 @@ void activateSignalingMemory()
return;

activateSignalHandlersFor(Signal::AccessFault);
WTF::finalizeSignalHandlers();
});
}

Expand All @@ -153,6 +139,9 @@ void prepareSignalingMemory()
if (!Options::useWasmFaultSignalHandler())
return;

#if CPU(ARM64E) && HAVE(HARDENED_MACH_EXCEPTIONS)
presignedTrampoline = g_wtfConfig.signalHandlers.presignReturnPCForHandler(LLInt::getCodePtr<NoPtrTag>(wasm_throw_from_fault_handler_trampoline_reg_instance));
#endif
addSignalHandler(Signal::AccessFault, [] (Signal signal, SigInfo& sigInfo, PlatformRegisters& ucontext) {
return trapHandler(signal, sigInfo, ucontext);
});
Expand Down
8 changes: 0 additions & 8 deletions Source/JavaScriptCore/wasm/WasmFaultSignalHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,4 @@ inline void activateSignalingMemory() { }
inline void prepareSignalingMemory() { }
#endif // ENABLE(WEBASSEMBLY)

#if CPU(ARM64E) && HAVE(HARDENED_MACH_EXCEPTIONS)
class MachExceptionSigningKey {
public:
uint32_t randomSigningKey = { };
MachExceptionSigningKey();
};
#endif // CPU(ARM64E) && HAVE(HARDENED_MACH_EXCEPTIONS)

} } // namespace JSC::Wasm
4 changes: 2 additions & 2 deletions Source/WTF/wtf/PlatformRegisters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ void* threadStateLRInternal(PlatformRegisters& regs)
void* threadStatePCInternal(PlatformRegisters& regs)
{
#if CPU(ARM64E) && HAVE(HARDENED_MACH_EXCEPTIONS)
// If userspace has modified the PC and set it to the presignedTrampoline,
// we want to avoid authing the value as it is using a custom ptrauth signing scheme.
// If we have modified the PC and set it to a presigned function we want to avoid
// authing the value as it is using a custom ptrauth signing scheme.
_STRUCT_ARM_THREAD_STATE64* ts = &(regs);
if (!(ts->__opaque_flags & __DARWIN_ARM_THREAD_STATE64_FLAGS_KERNEL_SIGNED_PC))
return nullptr;
Expand Down
3 changes: 0 additions & 3 deletions Source/WTF/wtf/Threading.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -500,9 +500,6 @@ void initialize()
#endif
initializeDates();
Thread::initializePlatformThreading();
#if USE(PTHREADS) && HAVE(MACHINE_CONTEXT)
SignalHandlers::initialize();
#endif
#if PLATFORM(COCOA)
initializeLibraryPathDiagnostics();
#endif
Expand Down
22 changes: 19 additions & 3 deletions Source/WTF/wtf/WTFConfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ void setPermissionsOfConfigPage()

void Config::initialize()
{
// FIXME: We should do a placement new for Config so we can use default initializers.
[]() -> void {
uintptr_t onePage = pageSize(); // At least, first one page must be unmapped.
#if OS(DARWIN)
Expand All @@ -124,14 +125,23 @@ void Config::initialize()
g_wtfConfig.lowestAccessibleAddress = onePage;
}();
g_wtfConfig.highestAccessibleAddress = static_cast<uintptr_t>((1ULL << OS_CONSTANT(EFFECTIVE_ADDRESS_WIDTH)) - 1);
SignalHandlers::initialize();
}

void Config::permanentlyFreeze()
void Config::finalize()
{
static Lock configLock;
Locker locker { configLock };
static std::once_flag once;
std::call_once(once, [] {
SignalHandlers::finalize();
if (!g_wtfConfig.disabledFreezingForTesting)
Config::permanentlyFreeze();
});
}

void Config::permanentlyFreeze()
{
RELEASE_ASSERT(roundUpToMultipleOf(pageSize(), ConfigSizeToProtect) == ConfigSizeToProtect);
ASSERT(!g_wtfConfig.disabledFreezingForTesting);

if (!g_wtfConfig.isPermanentlyFrozen) {
g_wtfConfig.isPermanentlyFrozen = true;
Expand Down Expand Up @@ -167,4 +177,10 @@ void Config::permanentlyFreeze()
RELEASE_ASSERT(g_wtfConfig.isPermanentlyFrozen);
}

void Config::disableFreezingForTesting()
{
RELEASE_ASSERT(!g_wtfConfig.isPermanentlyFrozen);
g_wtfConfig.disabledFreezingForTesting = true;
}

} // namespace WTF
3 changes: 3 additions & 0 deletions Source/WTF/wtf/WTFConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ constexpr size_t ConfigSizeToProtect = std::max(CeilingOnPageSize, 16 * KB);
struct Config {
WTF_EXPORT_PRIVATE static void permanentlyFreeze();
WTF_EXPORT_PRIVATE static void initialize();
WTF_EXPORT_PRIVATE static void finalize();
WTF_EXPORT_PRIVATE static void disableFreezingForTesting();

struct AssertNotFrozenScope {
AssertNotFrozenScope();
Expand All @@ -83,6 +85,7 @@ struct Config {
uintptr_t highestAccessibleAddress;

bool isPermanentlyFrozen;
bool disabledFreezingForTesting;
bool useSpecialAbortForExtraSecurityImplications;
#if PLATFORM(COCOA)
bool disableForwardingVPrintfStdErrToOSLog;
Expand Down

0 comments on commit b3eceeb

Please sign in to comment.