Closed Bug 1200740 Opened 9 years ago Closed 8 years ago

ASan crash due to child process function interceptions

Categories

(Core :: Security: Process Sandboxing, defect)

41 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: away, Assigned: ting)

References

Details

(Whiteboard: sb-)

Attachments

(1 file)

We create the plugin-container process in a suspended state, and set up some function interceptions in the child before it is allowed to run. This crashes under ASan because the NtOpenFile hook runs very early in the process, before ASan has initialized the shadow memory for the stack. What's the minimal thing that we have to disable to avoid this? Do we have to turn off the sandbox altogether? 1:059> r eax=0089aed9 ebx=00000208 ecx=00000000 edx=003cf5b0 esi=003cf6e0 edi=00079eb4 eip=0089af00 esp=003cf5a0 ebp=003cf754 iopl=0 nv up ei pl nz na pe nc cs=0023 ss=002b ds=002b es=002b fs=0053 gs=002b efl=00010206 plugin_container!TargetNtOpenFile+0x5f: 0089af00 c78700000030f1f104f2 mov dword ptr [edi+30000000h],0F204F1F1h ds:002b:30079eb4=???????? 1:059> k ChildEBP RetAddr 003cf754 77c2e057 plugin_container!TargetNtOpenFile+0x5f 003cf7c4 77c2eaf1 ntdll!RtlpCreateNewDirectoryReference+0x7b 003cf7dc 77c2d9a3 ntdll!RtlpInitCurrentDir+0x31 003cf97c 77c2ce55 ntdll!LdrpInitializeProcess+0xf88 003cf9cc 77c2a9c0 ntdll!_LdrpInitialize+0x90 003cf9d4 00000000 ntdll!LdrInitializeThunk+0x10
Bob is on PTO but he's the person who would know best.
Flags: needinfo?(bobowen.code)
(In reply to David Major [:dmajor] from comment #0) > We create the plugin-container process in a suspended state, and set up some > function interceptions in the child before it is allowed to run. This > crashes under ASan because the NtOpenFile hook runs very early in the > process, before ASan has initialized the shadow memory for the stack. > > What's the minimal thing that we have to disable to avoid this? Do we have > to turn off the sandbox altogether? Probably, these interceptions are always needed for the client end of the named pipes. If this is some sort of automated run, would we be able to set the env var MOZ_DISABLE_CONTENT_SANDBOX=1 for it? Depending on what is being tested you might need to set MOZ_DISABLE_GMP_SANDBOX and MOZ_DISABLE_NPAPI_SANDBOX as well.
Flags: needinfo?(bobowen.code)
No point in an env var if the build doesn't work without it. We might as well disable the sandbox at compile time ifdef MOZ_ASAN (and maybe Windows). I've discarded this objdir but I'll do some testing the next time I return to this build.
MOZ_DISABLE_CONTENT_SANDBOX=1 does seem to get past this problem, so I guess we could do the equivalent thing at build time.
(In reply to David Major [:dmajor] from comment #3) > No point in an env var if the build doesn't work without it. We might as > well disable the sandbox at compile time ifdef MOZ_ASAN (and maybe Windows). Linux sandboxing works differently and doesn't have this kind of problem (and so far I've been able to keep it working with ASan/LSan, although I've written off TSan as a lost cause), so that should be Windows-only. Also, I found https://crbug.com/382867 — it looks like there's a fix upstream, but it's newer than our current imported version. (https://crbug.com/484711 might also be relevant if we want to use sanitizer coverage, but if I understand correctly we currently don't.)
Whiteboard: sb-
Comment on attachment 8809296 [details] Bug 1200740 - Import blacklist for ASan on Windows to avoid instrumenting selected sandbox functions and files. https://reviewboard.mozilla.org/r/91878/#review92142
Attachment #8809296 - Flags: review?(mh+mozilla) → review+
Pushed by tchou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/75e62eb1d032 Import blacklist for ASan on Windows to avoid instrumenting selected sandbox functions and files. r=glandium
Assignee: nobody → janus926
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Blocks: 1317471
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: