From 9669df786cf4efe4430a5a8315cc9b8950237e75 Mon Sep 17 00:00:00 2001 From: Aaron Klotz Date: Thu, 26 Jul 2018 15:34:48 -0600 Subject: [PATCH] Bug 1478036: Ensure that inproc nop-space patches use atomic writes; r=handyman --HG-- extra : rebase_source : 9542cd801a8d4589e47d161c17c92552db468e7a --- mozglue/misc/interceptor/MMPolicies.h | 8 ++++ mozglue/misc/interceptor/PatcherNopSpace.h | 6 +-- mozglue/misc/interceptor/TargetFunction.h | 50 ++++++++++++++++++++++ 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/mozglue/misc/interceptor/MMPolicies.h b/mozglue/misc/interceptor/MMPolicies.h index d67ea01e94cd..30bc31701b09 100644 --- a/mozglue/misc/interceptor/MMPolicies.h +++ b/mozglue/misc/interceptor/MMPolicies.h @@ -122,6 +122,14 @@ public: return true; } +#if defined(_M_IX86) + bool WriteAtomic(void* aDestPtr, const uint16_t aValue) const + { + *static_cast(aDestPtr) = aValue; + return true; + } +#endif // defined(_M_IX86) + bool Protect(void* aVAddress, size_t aSize, uint32_t aProtFlags, uint32_t* aPrevProtFlags) const { diff --git a/mozglue/misc/interceptor/PatcherNopSpace.h b/mozglue/misc/interceptor/PatcherNopSpace.h index f158783f2e5c..e2397b1bea81 100644 --- a/mozglue/misc/interceptor/PatcherNopSpace.h +++ b/mozglue/misc/interceptor/PatcherNopSpace.h @@ -51,8 +51,7 @@ public: } // mov edi, edi - fn.WriteShort(0xff8b); - fn.Commit(); + fn.CommitAndWriteShort(0xff8b); } mPatchedFns.clear(); @@ -206,8 +205,7 @@ public: sizeof(uint16_t)); // Short jump up into our long jump. - writableFn.WriteShort(0xF9EB); // jmp $-5 - return writableFn.Commit(); + return writableFn.CommitAndWriteShort(0xF9EB); // jmp $-5 } }; diff --git a/mozglue/misc/interceptor/TargetFunction.h b/mozglue/misc/interceptor/TargetFunction.h index 66bfea49facd..9979d0a120a9 100644 --- a/mozglue/misc/interceptor/TargetFunction.h +++ b/mozglue/misc/interceptor/TargetFunction.h @@ -177,6 +177,9 @@ public: } mMMPolicy.FlushInstructionCache(); + + mStartWriteOffset += mLocalBytes.length(); + mLocalBytes.clear(); return true; } @@ -228,6 +231,53 @@ public: mOffset += sizeof(uint16_t); } +#if defined(_M_IX86) +private: + template + bool CommitAndWriteShortInternal(const T& aMMPolicy, void* aDest, uint16_t aValue); + + template <> + bool CommitAndWriteShortInternal(const MMPolicyInProcess& aMMPolicy, + void* aDest, uint16_t aValue) + { + return aMMPolicy.WriteAtomic(aDest, aValue); + } + + template <> + bool CommitAndWriteShortInternal(const MMPolicyOutOfProcess& aMMPolicy, + void* aDest, uint16_t aValue) + { + return aMMPolicy.Write(aDest, &aValue, sizeof(uint16_t)); + } + +public: + /** + * Commits any dirty writes, and then writes a short, atomically if possible. + * This call may succeed in both inproc and outproc cases, but atomicity + * is only guaranteed in the inproc case. + */ + bool CommitAndWriteShort(const uint16_t aValue) + { + // First, commit everything that has been written until now + if (!Commit()) { + return false; + } + + // Now immediately write the short, atomically if inproc + bool ok = CommitAndWriteShortInternal(mMMPolicy, + reinterpret_cast(mFunc + + mStartWriteOffset), + aValue); + if (!ok) { + return false; + } + + mMMPolicy.FlushInstructionCache(); + mStartWriteOffset += sizeof(uint16_t); + return true; + } +#endif // defined(_M_IX86) + void WriteDisp32(const uintptr_t aAbsTarget) { intptr_t diff = static_cast(aAbsTarget) -