1520310 - Sandbox broken at runtime on x86 Mingw-Clang build

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Content Process seems to have a problem. Tested with https://treeherder.mozilla.org/#/jobs?repo=try&revision=18c6aa91dd32be03d9f3f071ab63dcee4283b7b4 opt build

Comment 1

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: windbg output.txt

Comment 2

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

So I narrowed this down to enabling the sandbox. When I debug it; I'm breaking on this instruction:

cmp dword ptr [ntdll!LdrDelegatedRtlUserThreadStart (77bd77a4)],0

But I don't have a callstack and it's not clear to me what the problem is. The attached log is from an opt build; but the debug behavior is the same. Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=de1af56df4509aa548ec9e3ab11739d2f671394a

Comment 3

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: crash callstack.txt

Comment 4

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Looked at this a little bit and this seems like it's going to be an ugly one...

Comment 5

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

At present, this looks like it may be the same issue as Bug 1460882 - checking...

Comment 6

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1520310 - Disable SANDBOX_EXPORTS for the mingw-clang build as well r?bobowen

This fixed the x86 Build and doesn't affect the x64 build.

Comment 7

[Bob Owen (:bobowen)]

Comment on attachment 9038027 [details] [diff] [review]
Bug 1520310 - Disable SANDBOX_EXPORTS for the mingw-clang build as well r?bobowen

Review of attachment 9038027 [details] [diff] [review]:
-----------------------------------------------------------------

::: security/sandbox/moz.build
@@ +158,5 @@
>  
>      for var in ('UNICODE', '_UNICODE', 'NS_NO_XPCOM',
>                  '_CRT_RAND_S', 'CHROMIUM_SANDBOX_BUILD'):
>          DEFINES[var] = True
> +    if CONFIG['CC_TYPE'] != 'gcc' and CONFIG['CC_TYPE'] != 'clang':

I think it's slightly clearer to use:
if CONFIG['CC_TYPE'] not in ('gcc', 'clang'):

Comment 8

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1520310 - Disable SANDBOX_EXPORTS for the mingw-clang build as well r=bobowen

r+ from Comment 7

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8e4a86e306a
Disable SANDBOX_EXPORTS for the mingw-clang build as well r=bobowen

Comment 10

[Noemi Erli[:noemi_erli]]

Backed out changeset d8e4a86e306a (Bug 1520310) for bustages in filesystem_dispatcher.cc

Push with bustages: Backed out changeset d8e4a86e306a (Bug 1520310) for bustages in filesystem_dispatcher.cc

Bustage log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=223140101&repo=mozilla-inbound&lineNumber=12737

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/ed02a9881c0d43d5ab3e3040e878ed771dd1f10c

Comment 11

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1520310 - Backout the patch from Bug 1498695 and cast to void* r?bobowen

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/754481bfbe99
Backout the patch from Bug 1498695 and cast to void*. r=bobowen

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/754481bfbe99

Comment 14

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1520310 - Disable SANDBOX_EXPORTS for the mingw-clang build and enable it r=bobowen (esr60)

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: This patch backports the fix for the mingw-clang sandbox not running on x86 and enables the sandbox on both x86 and x64.

User impact if declined: Tor will need to carry an additional patch; and our -central and -esr60 branches will be out of sync which is confusing.

Fix Landed on Version:

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Only affects the mingw-clang build.

String or UUID changes made by this patch:

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f83bdfcf0804
Disable SANDBOX_EXPORTS for the mingw-clang build as well r=bobowen

Comment 16

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/f83bdfcf0804

Comment 17

[Julien Cristau [:jcristau]]

Comment on attachment 9038150 [details] [diff] [review]
Bug 1520310 - Disable SANDBOX_EXPORTS for the mingw-clang build and enable it r=bobowen (esr60)

enable sandbox for mingwclang builds, approved for 60.5esr build2

Comment 18

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/cbae43b53501