Closed Bug 1286119 Opened 5 years ago Closed 5 years ago

Seccomp sandbox violation: sys_mremap called in content process of Firefox desktop, when jemalloc disabled from --enable-trace-malloc

Categories

(Core :: Security: Process Sandboxing, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: dbaron, Assigned: tedd)

References

(Blocks 1 open bug)

Details

(Keywords: crash, Whiteboard: sblc1)

Attachments

(1 file)

I run Linux debug builds with --enable-trace-malloc, which implicitly disables jemalloc and instead uses the system allocator.

Sandboxing causes such builds not to start up anymore on Linux.  The error is:

Sandbox: seccomp sandbox violation: pid 4310, syscall 25, args 140165972488192 2101248 4198400 1 13 140168028284216.  Killing process.
Sandbox: crash reporter is disabled (or failed); trying stack trace:
Sandbox: frame #01: __GI___mremap (/build/glibc-GKVZIf/glibc-2.23/misc/../sysdeps/unix/syscall-template.S:84)
Sandbox: frame #02: mremap_chunk (/build/glibc-GKVZIf/glibc-2.23/malloc/malloc.c:2881)
Sandbox: frame #03: __GI___libc_realloc (/build/glibc-GKVZIf/glibc-2.23/malloc/malloc.c:3027)
Sandbox: frame #04: mozilla::detail::VectorImpl<unsigned char, 256ul, js::SystemAllocPolicy, true>::growTo(mozilla::Vector<unsigned char, 256ul, js::SystemAllocPolicy>&, unsigned long) (/home/dbaron/builds/ssd/mozilla-central/obj/firefox-
...
Try push for build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5849fa65b354

Apparently we already whitelist this call for Android.

:dbaron, could you apply this patch and see if it works? I hope the option also doesn't set MOZ_MEMORY.

:gcp and :jld, here [1] is a discussion about allowing sys_mremap, an interesting discussion starts here [2]. Apparently denying mremap with errno would cause the usage of a fallback. I looked into the glibc source [3] and the fallback code is still present.

They also discussed restricting the |flag| parameter to the value 0 for security reasons.

In the end they decided to just whitelist the call [4] from what I understand it was because of performance.

Before I request review for this, I wanted your opinions.

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=149834
[2] https://bugs.chromium.org/p/chromium/issues/detail?id=149834#c32
[3] http://osxr.org:8080/glibc/source/malloc/malloc.c#2993
[4] https://cs.chromium.org/chromium/src/content/common/sandbox_linux/bpf_renderer_policy_linux.cc?q=__NR_mremap&sq=package:chromium&l=76&dr=C
Assignee: nobody → julian.r.hector
Attachment #8770203 - Flags: feedback?(jld)
Attachment #8770203 - Flags: feedback?(gpascutto)
Attachment #8770203 - Flags: feedback?(dbaron)
See Also: → 1286308
Duplicate of this bug: 1286308
I just manually verified that attachment 8770203 [details] [diff] [review] fixes the problem for me.
Duplicate of this bug: 1286308
Duplicate of this bug: 1286308
Thank you :drno for checking.
Comment on attachment 8770203 [details] [diff] [review]
Allow sys_mremap when jemalloc is disabled. r=gcp

With this patch, I was able to start up without the crash, so it seems fixed.  (Though it was possible to start up with a relatively trivial session even without the patch, so I'm not 100% sure I'm testing the same steps, but the problem was presumably related to one of the pinned tabs.  And it certainly seems like this should fix it.)
Attachment #8770203 - Flags: feedback?(dbaron) → feedback+
Comment on attachment 8770203 [details] [diff] [review]
Allow sys_mremap when jemalloc is disabled. r=gcp

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

LGTM but can we file the flags restriction as a separate bug. Apparently Chrome never fixed it? I shot an email to the Chromium guys.
Attachment #8770203 - Flags: feedback?(jld)
Attachment #8770203 - Flags: feedback?(gpascutto)
Attachment #8770203 - Flags: feedback+
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/535e23baec4a
Add CASES_FOR_fchown and use it. r=gcp
(In reply to Pulsebot from comment #9)
> Pushed by cbook@mozilla.com:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/535e23baec4a
> Add CASES_FOR_fchown and use it. r=gcp

Why is this commented in this bug and not Bug 1286413 which handles the sys_fchown case?

Oh, I think I messed up the commit message in Bug 1286413.
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5789d5804cae
Backed out changeset 535e23baec4a for landing with wrong bugnumber
Whiteboard: sblc1
Attachment #8770203 - Flags: review?(gpascutto)
Comment on attachment 8770203 [details] [diff] [review]
Allow sys_mremap when jemalloc is disabled. r=gcp

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

LGTM but can we file the flags restriction as a separate bug. Apparently Chrome never fixed it? I shot an email to the Chromium guys.
Attachment #8770203 - Flags: review?(gpascutto) → review+
Blocks: 1288384
https://hg.mozilla.org/mozilla-central/rev/250943418f3a
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.