Closed Bug 1531030 Opened 7 months ago Closed 7 months ago

IOInterposer detours racing with launcher process background thread

Categories

(Core :: XPCOM, defect, P1, critical)

Unspecified
Windows 7
defect

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- disabled
firefox66 --- disabled
firefox67 --- fixed

People

(Reporter: calixte, Assigned: aklotz)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug is for crash report bp-26d1c33c-a516-476c-b759-47c240190227.

Top 10 frames of crashing thread:

0  @0x246 
1 kernelbase.dll GetFileAttributesExW 
2 wintrust.dll SortedCatalogOpen 
3 wintrust.dll CatalogLoadSortedFileData 
4 wintrust.dll alloca_probe 
5 wintrust.dll CatUtil_NoContentMsgDecode 
6 wintrust.dll I_CertDiagControl 
7 wintrust.dll alloca_probe 
8 wintrust.dll WinVerifyTrust 
9 mozglue.dll static bool `anonymous namespace'::SignedBinary::VerifySignatureInternal mozglue/build/Authenticode.cpp:184

There is 1 crash in nightly 67 with buildid 20190226215106. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1460433.

[1] https://hg.mozilla.org/mozilla-central/rev?node=7f034553129d

Flags: needinfo?(aklotz)
Crash Signature: [@ GetFileAttributesExW] → [@ GetFileAttributesExW] [@ CreateFileInternal]
Duplicate of this bug: 1531168
Crash Signature: [@ GetFileAttributesExW] [@ CreateFileInternal] → [@ GetFileAttributesExW] [@ CreateFileInternal] [@ LdrpMapResourceFile]
Flags: needinfo?(aklotz)

The fundamental problem here is that, when the launcher process fails, it spawns a background thread to log the failure to telemetry while allowing the main thread to continue to start as the browser process.

The IOInterposer attempts to hook NT file I/O system call stubs. Notice that all crash reports are only on x86-64. This is because we use an atomic NOP-space patch on x86, and we do not yet support these hooks on ARM64.

The launcher's background thread is attempting to issue I/O system calls concurrently with IOInterposer attempting to write detours to those functions. The launcher thread is then being sent off into the weeds.

I am clearing the regression keyword for a couple of reasons:

  • Yes, these crashes appeared after landing bug 1460433, but this is because of a bad interaction between two components; I technically did not regress an existing component;
  • IOInterposer is Nightly only.
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Component: General → XPCOM
Keywords: regression
Priority: P3 → P1
Product: Firefox → Core
Summary: Crash in [@ GetFileAttributesExW] → IOInterposer detours racing with launcher process background thread

If we are running a background thread in the launcher process to log failures,
then allowing the main thread to proceed with monkeypatching system calls is a
Bad Idea. This patch gives us an environment variable that, when set, indicates
that it is unsafe for PoisonIOInterposer to run.

This scenario is an uncommon one, but one that we must account for nonetheless.

Attachment #9047543 - Attachment description: Bug 1531030: Use MOZ_DISABLE_POISON_IO_INTERPOSER to disable PoisonIOInterposer when it is unsafe to initialize; r?froydnj → Bug 1531030: Use MOZ_DISABLE_POISON_IO_INTERPOSER to disable PoisonIOInterposer when it is unsafe to initialize; r?erahm
Pushed by aklotz@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7e2fced298cb
Use MOZ_DISABLE_POISON_IO_INTERPOSER to disable PoisonIOInterposer when it is unsafe to initialize; r=erahm
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Duplicate of this bug: 1532068
Crash Signature: [@ GetFileAttributesExW] [@ CreateFileInternal] [@ LdrpMapResourceFile] → [@ GetFileAttributesExW] [@ CreateFileInternal] [@ LdrpMapResourceFile] [@ ReadFile ]
You need to log in before you can comment on or make changes to this bug.