From 71a73f3c3b7b6feccaad25efee12a130abd97adf Mon Sep 17 00:00:00 2001 From: Luke Wagner Date: Mon, 24 Feb 2014 12:20:04 -0600 Subject: [PATCH] Bug 975182 - OdinMonkey: unprotect code while cloning (r=benj) --- js/src/jit-test/tests/asm.js/testBug975182.js | 18 +++++ js/src/jit/AsmJSModule.cpp | 77 ++++++++++++++++++- js/src/jit/AsmJSModule.h | 13 +++- js/src/jit/AsmJSSignalHandlers.cpp | 23 ++---- 4 files changed, 111 insertions(+), 20 deletions(-) create mode 100644 js/src/jit-test/tests/asm.js/testBug975182.js diff --git a/js/src/jit-test/tests/asm.js/testBug975182.js b/js/src/jit-test/tests/asm.js/testBug975182.js new file mode 100644 index 00000000000..3ff0bb5bd32 --- /dev/null +++ b/js/src/jit-test/tests/asm.js/testBug975182.js @@ -0,0 +1,18 @@ +Function("\ + g = (function(t,foreign){\ + \"use asm\";\ + var ff = foreign.ff;\ + function f() {\ + +ff()\ + }\ + return f\ + })(this, {\ + ff: arguments.callee\ + }, ArrayBuffer(4096))\ +")() +function m(f) { + for (var j = 0; j < 6000; ++j) { + f(); + } +} +m(g); diff --git a/js/src/jit/AsmJSModule.cpp b/js/src/jit/AsmJSModule.cpp index 1aad44118ef..c2aacb1fb67 100644 --- a/js/src/jit/AsmJSModule.cpp +++ b/js/src/jit/AsmJSModule.cpp @@ -833,9 +833,40 @@ AsmJSModule::deserialize(ExclusiveContext *cx, const uint8_t *cursor) return cursor; } -bool -AsmJSModule::clone(ExclusiveContext *cx, ScopedJSDeletePtr *moduleOut) const +// When a module is cloned, we memcpy its executable code. If, right before or +// during the clone, another thread calls AsmJSModule::protectCode() then the +// executable code will become inaccessible. In theory, we could take away only +// PROT_EXEC, but this seems to break emulators. +class AutoUnprotectCodeForClone { + JSRuntime *rt_; + JSRuntime::AutoLockForOperationCallback lock_; + const AsmJSModule &module_; + const bool protectedBefore_; + + public: + AutoUnprotectCodeForClone(JSContext *cx, const AsmJSModule &module) + : rt_(cx->runtime()), + lock_(rt_), + module_(module), + protectedBefore_(module_.codeIsProtected(rt_)) + { + if (protectedBefore_) + module_.unprotectCode(rt_); + } + + ~AutoUnprotectCodeForClone() + { + if (protectedBefore_) + module_.protectCode(rt_); + } +}; + +bool +AsmJSModule::clone(JSContext *cx, ScopedJSDeletePtr *moduleOut) const +{ + AutoUnprotectCodeForClone cloneGuard(cx, *this); + *moduleOut = cx->new_(scriptSource_, charsBegin_); if (!*moduleOut) return false; @@ -871,6 +902,48 @@ AsmJSModule::clone(ExclusiveContext *cx, ScopedJSDeletePtr *moduleO return true; } +void +AsmJSModule::protectCode(JSRuntime *rt) const +{ + JS_ASSERT(rt->currentThreadOwnsOperationCallbackLock()); + + // Technically, we should be able to only take away the execute permissions, + // however this seems to break our emulators which don't always check + // execute permissions while executing code. +#if defined(XP_WIN) + DWORD oldProtect; + if (!VirtualProtect(codeBase(), functionBytes(), PAGE_NOACCESS, &oldProtect)) + MOZ_CRASH(); +#else // assume Unix + if (mprotect(codeBase(), functionBytes(), PROT_NONE)) + MOZ_CRASH(); +#endif + + codeIsProtected_ = true; +} + +void +AsmJSModule::unprotectCode(JSRuntime *rt) const +{ +#if defined(XP_WIN) + DWORD oldProtect; + if (!VirtualProtect(codeBase(), functionBytes(), PAGE_EXECUTE_READWRITE, &oldProtect)) + MOZ_CRASH(); +#else // assume Unix + if (mprotect(codeBase(), functionBytes(), PROT_READ | PROT_WRITE | PROT_EXEC)) + MOZ_CRASH(); +#endif + + codeIsProtected_ = false; +} + +bool +AsmJSModule::codeIsProtected(JSRuntime *rt) const +{ + JS_ASSERT(rt->currentThreadOwnsOperationCallbackLock()); + return codeIsProtected_; +} + static bool GetCPUID(uint32_t *cpuId) { diff --git a/js/src/jit/AsmJSModule.h b/js/src/jit/AsmJSModule.h index 98788f11a1d..7b4b500d2f7 100644 --- a/js/src/jit/AsmJSModule.h +++ b/js/src/jit/AsmJSModule.h @@ -416,6 +416,11 @@ class AsmJSModule FunctionCountsVector functionCounts_; + // This field is accessed concurrently when triggering the operation + // callback and access must be sychronized via the runtime's operation + // callback lock. + mutable bool codeIsProtected_; + public: explicit AsmJSModule(ScriptSource *scriptSource, uint32_t charsBegin); ~AsmJSModule(); @@ -806,7 +811,13 @@ class AsmJSModule const uint8_t *deserialize(ExclusiveContext *cx, const uint8_t *cursor); bool loadedFromCache() const { return loadedFromCache_; } - bool clone(ExclusiveContext *cx, ScopedJSDeletePtr *moduleOut) const; + bool clone(JSContext *cx, ScopedJSDeletePtr *moduleOut) const; + + // These methods may only be called while holding the Runtime's operation + // callback lock. + void protectCode(JSRuntime *rt) const; + void unprotectCode(JSRuntime *rt) const; + bool codeIsProtected(JSRuntime *rt) const; }; // Store the just-parsed module in the cache using AsmJSCacheOps. diff --git a/js/src/jit/AsmJSSignalHandlers.cpp b/js/src/jit/AsmJSSignalHandlers.cpp index 040026306c0..61f841c1596 100644 --- a/js/src/jit/AsmJSSignalHandlers.cpp +++ b/js/src/jit/AsmJSSignalHandlers.cpp @@ -458,9 +458,7 @@ HandleException(PEXCEPTION_POINTERS exception) if (module.containsPC(faultingAddress)) { activation->setResumePC(pc); *ppc = module.operationCallbackExit(); - DWORD oldProtect; - if (!VirtualProtect(module.codeBase(), module.functionBytes(), PAGE_EXECUTE, &oldProtect)) - MOZ_CRASH(); + module.unprotectCode(rt); return true; } @@ -645,7 +643,7 @@ HandleMachException(JSRuntime *rt, const ExceptionRequest &request) const AsmJSModule &module = activation->module(); if (HandleSimulatorInterrupt(rt, activation, faultingAddress)) { - mprotect(module.codeBase(), module.functionBytes(), PROT_EXEC); + module.unprotectCode(rt); return true; } @@ -660,7 +658,7 @@ HandleMachException(JSRuntime *rt, const ExceptionRequest &request) if (module.containsPC(faultingAddress)) { activation->setResumePC(pc); *ppc = module.operationCallbackExit(); - mprotect(module.codeBase(), module.functionBytes(), PROT_EXEC); + module.unprotectCode(rt); // Update the thread state with the new pc. kret = thread_set_state(rtThread, x86_THREAD_STATE, (thread_state_t)&state, x86_THREAD_STATE_COUNT); @@ -892,7 +890,7 @@ HandleSignal(int signum, siginfo_t *info, void *ctx) const AsmJSModule &module = activation->module(); if (HandleSimulatorInterrupt(rt, activation, faultingAddress)) { - mprotect(module.codeBase(), module.functionBytes(), PROT_EXEC); + module.unprotectCode(rt); return true; } @@ -907,7 +905,7 @@ HandleSignal(int signum, siginfo_t *info, void *ctx) if (module.containsPC(faultingAddress)) { activation->setResumePC(pc); *ppc = module.operationCallbackExit(); - mprotect(module.codeBase(), module.functionBytes(), PROT_EXEC); + module.unprotectCode(rt); return true; } @@ -1027,16 +1025,7 @@ js::TriggerOperationCallbackForAsmJSCode(JSRuntime *rt) if (!activation) return; - const AsmJSModule &module = activation->module(); - -#if defined(XP_WIN) - DWORD oldProtect; - if (!VirtualProtect(module.codeBase(), module.functionBytes(), PAGE_NOACCESS, &oldProtect)) - MOZ_CRASH(); -#else // assume Unix - if (mprotect(module.codeBase(), module.functionBytes(), PROT_NONE)) - MOZ_CRASH(); -#endif + activation->module().protectCode(rt); } #if defined(MOZ_ASAN) && defined(JS_STANDALONE)