Closed Bug 1520310 Opened 10 months ago Closed 10 months ago

Sandbox broken at runtime on x86 Mingw-Clang build

Categories

(Core :: Security: Process Sandboxing, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- fixed
firefox66 --- fixed

People

(Reporter: tjr, Assigned: tjr)

References

(Blocks 1 open bug)

Details

(Whiteboard: [tor])

Attachments

(4 files, 2 obsolete files)

Component: General: Unsupported Platforms → Security: Process Sandboxing
Product: Firefox Build System → Core
Summary: x86 Mingw-Clang builds do not run → Sandbox broken at runtime on x86 Mingw-Clang build

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

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

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

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

Attachment #9038027 - Flags: review?(bobowencode)
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'):
Attachment #9038027 - Flags: review?(bobowencode) → review+

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8e4a86e306a
Disable SANDBOX_EXPORTS for the mingw-clang build as well r=bobowen

Keywords: checkin-needed

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

Flags: needinfo?(tom)
Attachment #9038051 - Flags: review?(bobowencode) → review+

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

Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

[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:

Attachment #9038150 - Flags: approval-mozilla-esr60?
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f83bdfcf0804
Disable SANDBOX_EXPORTS for the mingw-clang build as well r=bobowen
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
Attachment #9038150 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
You need to log in before you can comment on or make changes to this bug.