In the fx codebase, we tend to use __attribute__((naked)) for plain
assembly functions with a C interface. Instrumenting these functions can
mess up this assembly, so we conservatively also want to prevent
instrumentation.
Differential Revision: https://phabricator.services.mozilla.com/D196154
Otherwise injected code, under temporal profiling, messes up the inline
assembly used by the wrapper. Ideally, we would use extended asm to
correctly state the constraint, but it is incompatible with naked
functions.
See discussion in https://github.com/llvm/llvm-project/issues/74573 for
the interaction between ``naked`` and profiling.
Differential Revision: https://phabricator.services.mozilla.com/D195315
We currently fail to guarantee that OnEndDllLoad is called on the same
gLoaderObserver as OnBeginDllLoad. We must implement additional
synchronization to prevent a race condition where a call to
LoaderPrivateAPIImp::SetObserver would come in between the two and
change gLoaderObserver.
This has led to issues when using MOZ_PROFILER_STARTUP=1 where we would
have sStackWalkSuppressions reach (size_t)-1 instead of 0, later
resulting in deadlock or missing stacks. See bug 1687510 comment 10 for
extra details.
Depends on D181436
Differential Revision: https://phabricator.services.mozilla.com/D181437
When a first hang is detected, the BHMgr Monitor thread needs to commit
5 more pages of stack to run profiler_suspend_and_sample_thread, which
contains big stack variables. If that occurs while we are low on memory,
failure to commit stack pages can crash the process.
In bug 1716727, we have added delays on failed allocations to try
to avoid crashing the main process under low memory condition. These
delays could trigger the background hang monitor, which could in
turn crash the process, as they occur in a low memory condition where we
will likely fail to commit.
We can pre-commit the 5 pages of stack at thread initialization to
ensure that they will already be commited when we later need them. Or at
least, we can try to see if that works.
We do that with a wrapper for the __chkstk function. We add a new test
in the NativeNt cppunit test, to ensure that our wrapper function
behaves as expected.
Differential Revision: https://phabricator.services.mozilla.com/D182582
With bug 1832467 we have updated our Windows SDK version to 10.0.19041.
As a result, we now have a .retplne section in xul.dll, starting with
Firefox 115. This is a section with PAGE_NOACCESS protection, so
accessing it crashes the process.
Some injected DLLs read the whole memory space dedicated to the xul.dll
image to search for patterns in it. When they hit the .retplne section,
we will crash. This happened for a legit product in bug 1837242, but
also for a malicious DLL in bug 1841751. This is a startup crash.
This changeset blocks the variants of this malicious DLL we know, to
eliminate the associated startup crash spike. Because the DLL does not
use a fixed name, we block by matching on the combination of version
number + timestamp + image size, based on the values found in crash
reports. We additionnally check for a checksum of 0 and the absence of
debug information, both of which are uncommon for legit production-ready
DLLs; this thus helps further reduce the chances of collision.
Differential Revision: https://phabricator.services.mozilla.com/D183096
While fixing a crash in bug 1733532, we accidentally broke the DLL
blocklist on older versions of Windows (Windows 7, some versions
of Windows 10, and possibly Windows 8 and 8.1). This is currently
preventing us from mitigating crashes with third-party injected DLLs, in
particular the crash incident from bug 1837242. Considering the volumes
involved, let's temporarily reintroduce bug 1733532 to ensure everyone
has a working blocklist, and deal with bug 1733532 later.
Differential Revision: https://phabricator.services.mozilla.com/D182917
We had crashes in `PEHeaders::FindResourceLeaf` where `idDir` was nullptr. This can
happen when the resource table is modified by a third-party application and an entry
in the table points to somewhere outside the executable.
Differential Revision: https://phabricator.services.mozilla.com/D121093
`SharedLibraryInfo::GetInfoForSelf()` can use `PEHeaders::GetBounds` instead of
`GetModuleInformation` to get the start/end address of a module's mapped region
in the local process. It's roughly 100x faster because `GetModuleInformation`
invokes two system calls `NtQueryInformationProcess` and `NtReadVirtualMemory`
while `nt::PEHeaders` does not.
Depends on D115254
Differential Revision: https://phabricator.services.mozilla.com/D115255
This patch is the actual fix for Bug 1702086. The problem of Bug 1702086 is that
`LoadLibraryExW` loaded the module onto an address different from the original
mapped addresss because it was unloaded after we started enumeration. Calling
`GetPdbInfo` with the original address `module.lpBaseOfDll` caused a crash.
The proposed fix consists of three parts.
The first part is to get PDB information from `handleLock`, which is always valid
even if the original address was unloaded. With this, we don't need a check
of `VirtualQuery`.
The second part is to add `LOAD_LIBRARY_AS_IMAGE_RESOURCE` along with
`LOAD_LIBRARY_AS_DATAFILE` to the call to `LoadLibraryEx`. This is needed
to read information from the sections outside the PE headers because
RVA (= relative virtual address) is an address after relocation.
Without `LOAD_LIBRARY_AS_IMAGE_RESOURCE`, a module is mapped without relocation,
so `GetPdbInfo()` accesses wrong memory resulting in a crash.
The third part is to introduce `PEHeaders::GetPdbInfo`, replacing two versions
of `GetPdbInfo` in Gecko profiler and baseprofiler.
Depends on D115252
Differential Revision: https://phabricator.services.mozilla.com/D115253
Bug 1620118 added a new field `isDependent` in the third-party-module ping
which is calculated in `UntrustedModulesProcessor`. However, bug 1684532
revealed it was not accurate because some third-party applications revert
the import table to the original state immediately after their module was
loaded.
Now that we have a logic to determine `isDependent` in `NtMapViewOfSection`
to automatically block a module injected through the import table, we can
pass that value to the ping and remove the original logic in `UntrustedModulesProcessor`.
Differential Revision: https://phabricator.services.mozilla.com/D112227
This patch is to improve the way to detect an injected dependent module for
automatic DLL blocking (bug 1659438).
In the previous version, we created a list of dependent modules in the launcher
process and shared it with other processes via the shared section. However, it
was not compatible with third-party applications who tamper the Import Table and
revert it in the injected module's DllMain (bug 1682834) because we parsed the
Import Table in the launcher process after it was reverted.
With this patch, we check the Import Table in `patched_NtMapViewOfSection`,
so we can see tampering before it's reverted. More specifically, we create
a list of dependent modules in the browser process as below.
1. The launcher process creates a section object and initializes
the kernel32.dll's functions in it.
2. The launcher process transfers a writable handle of the shared
section to the browser process.
3. In the browser process, if an injected dependent module is being
mapped by `NtMapViewOfSection`, we add its NT path to the shared
section and block it with `REDIRECT_TO_NOOP_ENTRYPOINT`.
4. The `main` function of the browser process converts the writable
handle of the shared section into a readonly handle.
5. The browser process transfers a readonly handle of the shared
section to a sandbox process.
Since automatic DLL blocking may still cause a compat issue like bug 1682304,
we activate it only in Nightly for now.
Differential Revision: https://phabricator.services.mozilla.com/D101460
This patch is to improve the way to detect an injected dependent module for
automatic DLL blocking (bug 1659438).
In the previous version, we created a list of dependent modules in the launcher
process and shared it with other processes via the shared section. However, it
was not compatible with third-party applications who tamper the Import Table and
revert it in the injected module's DllMain (bug 1682834) because we parsed the
Import Table in the launcher process after it was reverted.
With this patch, we check the Import Table in `patched_NtMapViewOfSection`,
so we can see tampering before it's reverted. More specifically, we create
a list of dependent modules in the browser process as below.
1. The launcher process creates a section object and initializes
the kernel32.dll's functions in it.
2. The launcher process transfers a writable handle of the shared
section to the browser process.
3. In the browser process, if an injected dependent module is being
mapped by `NtMapViewOfSection`, we add its NT path to the shared
section and block it with `REDIRECT_TO_NOOP_ENTRYPOINT`.
4. The `main` function of the browser process converts the writable
handle of the shared section into a readonly handle.
5. The browser process transfers a readonly handle of the shared
section to a sandbox process.
Since automatic DLL blocking may still cause a compat issue like bug 1682304,
we activate it only in Nightly for now.
Differential Revision: https://phabricator.services.mozilla.com/D101460
This patch adds a list of the executable's dependent module's path to SharedSection
as an array of the offset to a string and a string buffer. A following patch will
use this data from the browser process and the sandboxed processes.
Depends on D96283
Differential Revision: https://phabricator.services.mozilla.com/D96284
We transfer several ntdll's function addresses to a child process directly via
`WriteProcessMemory`. This patch changes the way to transfer data to using
a section object as Chromium sandbox does, so that we can transfer more data
with the same cost as transferring a single handle value.
Depends on D96282
Differential Revision: https://phabricator.services.mozilla.com/D96283
This patch introduces a class `CrossExecTransferManager` to manage the data
transfer from the current process to a remote process via `WriteProcessMemory`.
The class also encapsulates a logic to bridge the gap between two executable's
imagebase.
Differential Revision: https://phabricator.services.mozilla.com/D94652
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 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 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 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
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
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
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
This was done by:
This was done by applying:
```
diff --git a/python/mozbuild/mozbuild/code-analysis/mach_commands.py b/python/mozbuild/mozbuild/code-analysis/mach_commands.py
index 789affde7bbf..fe33c4c7d4d1 100644
--- a/python/mozbuild/mozbuild/code-analysis/mach_commands.py
+++ b/python/mozbuild/mozbuild/code-analysis/mach_commands.py
@@ -2007,7 +2007,7 @@ class StaticAnalysis(MachCommandBase):
from subprocess import Popen, PIPE, check_output, CalledProcessError
diff_process = Popen(self._get_clang_format_diff_command(commit), stdout=PIPE)
- args = [sys.executable, clang_format_diff, "-p1", "-binary=%s" % clang_format]
+ args = [sys.executable, clang_format_diff, "-p1", "-binary=%s" % clang_format, '-sort-includes']
if not output_file:
args.append("-i")
```
Then running `./mach clang-format -c <commit-hash>`
Then undoing that patch.
Then running check_spidermonkey_style.py --fixup
Then running `./mach clang-format`
I had to fix four things:
* I needed to move <utility> back down in GuardObjects.h because I was hitting
obscure problems with our system include wrappers like this:
0:03.94 /usr/include/stdlib.h:550:14: error: exception specification in declaration does not match previous declaration
0:03.94 extern void *realloc (void *__ptr, size_t __size)
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/malloc_decls.h:53:1: note: previous declaration is here
0:03.94 MALLOC_DECL(realloc, void*, void*, size_t)
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/mozilla/mozalloc.h:22:32: note: expanded from macro 'MALLOC_DECL'
0:03.94 MOZ_MEMORY_API return_type name##_impl(__VA_ARGS__);
0:03.94 ^
0:03.94 <scratch space>:178:1: note: expanded from here
0:03.94 realloc_impl
0:03.94 ^
0:03.94 /home/emilio/src/moz/gecko-2/obj-debug/dist/include/mozmemory_wrap.h:142:41: note: expanded from macro 'realloc_impl'
0:03.94 #define realloc_impl mozmem_malloc_impl(realloc)
Which I really didn't feel like digging into.
* I had to restore the order of TrustOverrideUtils.h and related files in nss
because the .inc files depend on TrustOverrideUtils.h being included earlier.
* I had to add a missing include to RollingNumber.h
* Also had to partially restore include order in JsepSessionImpl.cpp to avoid
some -WError issues due to some static inline functions being defined in a
header but not used in the rest of the compilation unit.
Differential Revision: https://phabricator.services.mozilla.com/D60327
--HG--
extra : moz-landing-system : lando
rg -l 'mozilla/Move.h' | xargs sed -i 's/#include "mozilla\/Move.h"/#include <utility>/g'
Further manual fixups and cleanups to the include order incoming.
Differential Revision: https://phabricator.services.mozilla.com/D60323
--HG--
extra : moz-landing-system : lando
serge-sans-paille
Yannis Juglaret
Nazım Can Altınova
Toshihito Kikuchi
Alexandru Michis
Butkovits Atila
smolnar
Simon Giesecke
Chris Peterson
Daniel Varga
Cosmin Sabou
Narcis Beleuzu
Emilio Cobos Álvarez