If EAF+ is enabled for firefox.exe, the process does not launch because we parse
the PE headers of ntdll.dll at startup, which is prohibited by EAF+.
With this patch, we skip two operations when EAF+ is enabled.
The first one is to cache ntdll's IAT at startup. Because EAF+ is expected to
prevent an injected module from parsing PE headers and modifying IAT, we can skip
this caching safely.
The second one is to load ntdll's debug information for the profiler. With this
patch, the profiler's callstack will not show a raw address instead of a symbol
name. It's a bad side effect, but much better than startup crash.
Differential Revision: https://phabricator.services.mozilla.com/D76959
This patch adds a boolean field `mIsDependent` indicating whether a module was
loaded via the executable's Import Directory Table or not.
This patch also partially reverts Bug 1587539, moving a logic to detect Import
Directory tampering to `PEHeaders`'s ctor. With this, we can skip generating
a map of the executable's dependent modules if no tampering is detected.
Differential Revision: https://phabricator.services.mozilla.com/D66274
Also move MOZ_MUST_USE before function declarations' specifiers and return type. While clang and gcc's __attribute__((warn_unused_result)) can appear before, between, or after function specifiers and return types, the [[nodiscard]] attribute must precede the function specifiers.
Differential Revision: https://phabricator.services.mozilla.com/D70631
--HG--
extra : moz-landing-system : lando
This commit removes `test_fix_stack_using_bpsyms.py`. That test can't easily be
modified to work with `fix_stacks.py` because it relies on internal
implementation details of `fix_stack_using_bpsym.py`. The unit testing done in
the `fix-stacks` repo provides test coverage that is as good or better.
Differential Revision: https://phabricator.services.mozilla.com/D66924
--HG--
extra : moz-landing-system : lando
Please double check that I am using this correctly. I believe we are
seeing the crash in the linked bug because we are not handling hardware
faults when reading from the memory mapped file. This patch just wraps
all accesses in the MMAP_FAULT_HANDLER_ macros.
Depends on D53042
Differential Revision: https://phabricator.services.mozilla.com/D53043
--HG--
rename : modules/libjar/MmapFaultHandler.cpp => mozglue/misc/MmapFaultHandler.cpp
rename : modules/libjar/MmapFaultHandler.h => mozglue/misc/MmapFaultHandler.h
extra : moz-landing-system : lando
This patch introduces `Kernel32ExportsSolver` which calculates RVAs of
kernel32's functions and transfers them to a target process, where the
transferred RVAs are resolved into function addresses.
Depends on D68346
Differential Revision: https://phabricator.services.mozilla.com/D68347
--HG--
extra : moz-landing-system : lando
This patch introduces a new DLL interceptor `WindowsDllEntryPointInterceptor`
which applies a hook to a target function without backing up the original
function code.
Depends on D68345
Differential Revision: https://phabricator.services.mozilla.com/D68346
--HG--
extra : moz-landing-system : lando
This patch introduces a new policy `MMPolicyInProcessEarlyStage` which does
not consume any functions imported from kernel32.dll so that we can use it
in a process's early stage i.e. before IAT is resolved.
Depends on D68344
Differential Revision: https://phabricator.services.mozilla.com/D68345
--HG--
extra : moz-landing-system : lando
`WindowsDllDetourPatcher::CreateTrampoline` does not only create a trampoline
region but also applies a patch on an original function. This patch extracts
the patching part as separate functions.
Differential Revision: https://phabricator.services.mozilla.com/D68344
--HG--
extra : moz-landing-system : lando
This patch moves the instantiation of `PEHeaders` from `CheckBlockInfo` to
`IsDllAllowed` so that `IsDllAllowed` can use an instance of `PEHeaders`.
Depends on D68342
Differential Revision: https://phabricator.services.mozilla.com/D68343
--HG--
extra : moz-landing-system : lando
This patch introduces `nt::VirtualQuery` which consumes only ntdll's functions
to reduce dependency in `MMPolicy` on kernel32.dll. With this, `MMPolicy` still
depends on kernel32.dll, that will be solved by a coming patch.
Differential Revision: https://phabricator.services.mozilla.com/D68342
--HG--
extra : moz-landing-system : lando
This patch introduces `Kernel32ExportsSolver` which calculates RVAs of
kernel32's functions and transfers them to a target process, where the
transferred RVAs are resolved into function addresses.
Depends on D68346
Differential Revision: https://phabricator.services.mozilla.com/D68347
--HG--
extra : moz-landing-system : lando
This patch introduces a new DLL interceptor `WindowsDllEntryPointInterceptor`
which applies a hook to a target function without backing up the original
function code.
Depends on D68345
Differential Revision: https://phabricator.services.mozilla.com/D68346
--HG--
extra : moz-landing-system : lando
This patch introduces a new policy `MMPolicyInProcessEarlyStage` which does
not consume any functions imported from kernel32.dll so that we can use it
in a process's early stage i.e. before IAT is resolved.
Depends on D68344
Differential Revision: https://phabricator.services.mozilla.com/D68345
--HG--
extra : moz-landing-system : lando
`WindowsDllDetourPatcher::CreateTrampoline` does not only create a trampoline
region but also applies a patch on an original function. This patch extracts
the patching part as separate functions.
Depends on D68343
Differential Revision: https://phabricator.services.mozilla.com/D68344
--HG--
extra : moz-landing-system : lando
This patch moves the instantiation of `PEHeaders` from `CheckBlockInfo` to
`IsDllAllowed` so that `IsDllAllowed` can use an instance of `PEHeaders`.
Depends on D68342
Differential Revision: https://phabricator.services.mozilla.com/D68343
--HG--
extra : moz-landing-system : lando
This patch introduces `nt::VirtualQuery` which consumes only ntdll's functions
to reduce dependency in `MMPolicy` on kernel32.dll. With this, `MMPolicy` still
depends on kernel32.dll, that will be solved by a coming patch.
Differential Revision: https://phabricator.services.mozilla.com/D68342
--HG--
extra : moz-landing-system : lando
When our detour processes instructions, we pass `ReadOnlyTargetFunction` to
`CountPrefixBytes` to determine whether a lock prefix exists or not.
In that case, we don't need to pass both `ReadOnlyTargetFunction` and an offset
as a parameter because `ReadOnlyTargetFunction` has an offset as a member.
Differential Revision: https://phabricator.services.mozilla.com/D69360
--HG--
extra : moz-landing-system : lando
Please double check that I am using this correctly. I believe we are
seeing the crash in the linked bug because we are not handling hardware
faults when reading from the memory mapped file. This patch just wraps
all accesses in the MMAP_FAULT_HANDLER_ macros.
Depends on D53042
Differential Revision: https://phabricator.services.mozilla.com/D53043
--HG--
rename : modules/libjar/MmapFaultHandler.cpp => mozglue/misc/MmapFaultHandler.cpp
rename : modules/libjar/MmapFaultHandler.h => mozglue/misc/MmapFaultHandler.h
extra : moz-landing-system : lando
Also adds missing includes in some files, these were previously only transivitely
included through mozilla/TypeTraits.h.
Differential Revision: https://phabricator.services.mozilla.com/D68561
--HG--
extra : moz-landing-system : lando
When we are running from a network drive the new feature in part 1 doesn't work.
So this uses DuplicateHandle instead of OpenThread to get the thread handle used
by the profiler.
It also removes a DuplicateHandle THREAD_ALL_ACCESS call that also fails and a
DuplicateHandle to get a real process handle, which only seems to have been to
fix something on Windows XP.
The handle passed in is always the profiler one, so already has the necessary
permissions. If no thread handle is passed then the pseudo handle is used.
Differential Revision: https://phabricator.services.mozilla.com/D66611
--HG--
extra : moz-landing-system : lando
When we are running from a network drive the new feature in part 1 doesn't work.
So this uses DuplicateHandle instead of OpenThread to get the thread handle used
by the profiler.
It also removes a DuplicateHandle THREAD_ALL_ACCESS call that also fails and a
DuplicateHandle to get a real process handle, which only seems to have been to
fix something on Windows XP.
The handle passed in is always the profiler one, so already has the necessary
permissions. If no thread handle is passed then the pseudo handle is used.
Depends on D66610
Differential Revision: https://phabricator.services.mozilla.com/D66611
--HG--
extra : moz-landing-system : lando
In the current Win 8.0, these functions both start with a RIP-relative JMP (6 bytes) followed by 6 nops (6-bytes), which does not give us the 13-bytes we need for a trampoline so we require the trampoline to fit into 10 bytes.
Differential Revision: https://phabricator.services.mozilla.com/D63260
--HG--
extra : moz-landing-system : lando
We copy IAT for ntdll.dll into a new process so that our hook code can use
ntdll's functions even in the early stage. However, IAT can be modified and
some entries may point to an address which is not valid in the child process.
In such a case, we should not copy IAT. One example is Windows compat mode
which redirects some ntdll functions into AcLayers.dll via IAT.
With this patch, we verify each IAT entry and if any of them is outside ntdll,
we give up using the launcher process and start the browser process.
Differential Revision: https://phabricator.services.mozilla.com/D62852
--HG--
extra : moz-landing-system : lando
In x86, our detour handles opcode 83 only when the Mod bits is 3.
When working on another project, I hit the instruction `cmp [ebp+0Ch],1`
where the Mod bits is 1, and it can be easily handled by a small fix.
It turned out my project does not need it, but it'd be good to have this.
Differential Revision: https://phabricator.services.mozilla.com/D64196
--HG--
extra : moz-landing-system : lando
This patch adds a function to get an exported function in a remote process.
We need this implementation to address Bug 1604008, Bug 1608645, and Bug 1610790.
When `WindowsDllInterceptor` detours a function in a remote process, we used the
native `GetProcAddress` locally, and then detours the returned address in the
target process. The problem is if the caller's export table was modified, the
address returned from `GetProcAddress` might be invalid in the target process,
which is Bug 1604008.
I implemented `GetProcAddress` depending on both local and remote process image,
but it caused two regressions Bug 1608645 and Bug 1610790 because multiple
applications modify firefox's export table in multiple ways, such as replacing
an entry of EAT, replacing an RVA to Export section, or etc.
With this patch, we can use `PEExportSection<MMPolicy>::GetProcAddress` to get
an exported function in a remote process without relying on any local data so
that it's not impacted by modification of the local export table.
Differential Revision: https://phabricator.services.mozilla.com//D62315
Depends on D62314
This patch adds a function to get an exported function in a remote process.
We need this implementation to address Bug 1604008, Bug 1608645, and Bug 1610790.
When `WindowsDllInterceptor` detours a function in a remote process, we used the
native `GetProcAddress` locally, and then detours the returned address in the
target process. The problem is if the caller's export table was modified, the
address returned from `GetProcAddress` might be invalid in the target process,
which is Bug 1604008.
I implemented `GetProcAddress` depending on both local and remote process image,
but it caused two regressions Bug 1608645 and Bug 1610790 because multiple
applications modify firefox's export table in multiple ways, such as replacing
an entry of EAT, replacing an RVA to Export section, or etc.
With this patch, we can use `PEExportSection<MMPolicy>::GetProcAddress` to get
an exported function in a remote process without relying on any local data so
that it's not impacted by modification of the local export table.
Differential Revision: https://phabricator.services.mozilla.com/D62315
Depends on D62314
--HG--
extra : rebase_source : 3088f5997a2097ef22ce8567783375e5f7866ab2
We had a thread-local varialbe `ModuleLoadFrame::sTopFrame` to track the topmost
stack frame of `LdrLoadDll`. However, our hook function `patched_LdrLoadDll` can
be called even before TLS is initialized. In such a case, accessing `sTopFrame`
causes AV.
This patch introduces `SafeThreadLocal` to safely access a thread-local varialbe.
If TLS is not initialized, it falls back to a global variable because in that
early stage there is only a single thread running.
Differential Revision: https://phabricator.services.mozilla.com/D55870
--HG--
extra : moz-landing-system : lando
We had a thread-local varialbe `ModuleLoadFrame::sTopFrame` to track the topmost
stack frame of `LdrLoadDll`. However, our hook function `patched_LdrLoadDll` can
be called even before TLS is initialized. In such a case, accessing `sTopFrame`
causes AV.
This patch introduces `SafeThreadLocal` to safely access a thread-local varialbe.
If TLS is not initialized, it falls back to a global variable because in that
early stage there is only a single thread running.
Differential Revision: https://phabricator.services.mozilla.com/D55870
--HG--
extra : moz-landing-system : lando
Under the stronger Control Flow Guard scheme coming in clang 10, when a nop-space hook jumps back to the original API, at `ntdll!Whatever+2`, that address is not a registered jump target, so we crash with a CFG failure. Since this is a deliberate violation of the rules, let's disable CFG for these calls.
Based on my testing, this is the only place we need to use this attribute, so I placed its definition close to the use. (Had we needed more of these, I would have put it in mfbt/.)
Differential Revision: https://phabricator.services.mozilla.com/D59728
--HG--
extra : moz-landing-system : lando
A third-party application can modify the export directory, the export address/name/ordinal
tables, or an entry in those tables. If that happens, we will see an RVA is located outside
the mapped image and `RVAToPtr` returns null. This patch makes sure we don't hit null AV
when modification is detected.
`FindExportAddressTableEntry` should not return a pointer to the modified table entry because
we dereference it in another process to cross-process detour.
Differential Revision: https://phabricator.services.mozilla.com/D59738
--HG--
extra : moz-landing-system : lando
Toshihito Kikuchi
Greg V
Chris Peterson
Nicholas Nethercote
Doug Thayer
Daniel Varga
Dorel Luca
André Bargull
Sylvestre Ledru
Markus Stange
Bob Owen
Narcis Beleuzu
Simon Giesecke
David Parks
Andrew McCreight
Ciure Andrei
Cosmin Sabou
David Major