Closed Bug 1041886 Opened 5 years ago Closed 5 years ago

Separate Linux sandbox code into its own shared library

Categories

(Core :: Security, defect)

All
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: jld, Assigned: jld)

References

Details

Attachments

(2 files)

Windows Firefox builds the sandbox code as its own shared library so that it can use versions of the chromium/base code that would otherwise conflict with those from IPC.  (See also bug 925471.)  We're going to need this on Linux if we want to import any more of the Chromium sandboxing code there (and we do, in fact, want that).

It will also allow getting rid of the unsightly hacks at the bottom of security/sandbox/linux/Sandbox.cpp (or, at least, allow sharing the equivalent of those hacks with other OSes — see security/sandbox/chromium/base/shim/base.)
Step 0: break out the logging macro into a header file, because it will be needed in more than one module in the next patch.  Also rename it, because having something with as generic a name as LOG_ERROR in an include file seems not right.
Attachment #8474930 - Flags: review?(gdestuynder)
…and this is the patch that actually moves code around and creates the separate library for the sandbox.

I'm not sure if this needs build peer review, but it probably can't hurt to ask for it, especially because this is the first time I've tried to do something like this to the build config.
Attachment #8474931 - Flags: review?(mh+mozilla)
Attachment #8474931 - Flags: review?(gdestuynder)
Comment on attachment 8474931 [details] [diff] [review]
bug1041886-p1-sandbox-lib-hg1.diff

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

::: security/sandbox/linux/glue/moz.build
@@ +5,5 @@
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +FAIL_ON_WARNINGS = True
> +
> +SOURCES += ['SandboxCrash.cpp']

SOURCES += [
    'SandboxCrash.cpp',
]

same for others
Attachment #8474931 - Flags: review?(mh+mozilla) → review+
Changeset 2f9d0821e08c breaks the build on Linux:

 0:17.04 ../../../build/unix/gold/ld: error: /home/kinetik/mozilla/inbound/obj-x86_64-unknown-linux-gnu/security/sandbox/linux/Sandbox.o: requires dynamic R_X86_64_PC32 reloc against '_ZN7mozilla6unusedE' which may overflow at runtime; recompile with -fPIC
 0:17.04 ../../../build/unix/gold/ld: error: read-only segment has dynamic relocations
 0:17.04 /home/kinetik/mozilla/inbound/security/sandbox/linux/Sandbox.cpp:403: error: undefined reference to 'mozilla::unused'
 0:17.04 collect2: error: ld returned 1 exit status
I also hit this link error on my Linux box.
Sounds like a visibility issue.
I can't reproduce, fwiw. What's your version of gold? (gold --version)
Are the failing builds -O0, maybe?  That might explain why I have no actual references to mozilla::unused, and neither my builds nor try/buildbot had problems.

For some reason I thought mozilla::unused was MFBT, but it's actually xpcom/glue.  In any case, this *should* be a one-line fix.
(In reply to Mike Hommey [:glandium] (out from Sep 6 to Sep 22) from comment #8)
> I can't reproduce, fwiw. What's your version of gold? (gold --version)

~% gold --version
zsh: command not found: gold
[127] ~% ld.gold --version
GNU gold (version 2.23.2) 1.11

(In reply to Jed Davis [:jld] from comment #9)
> Are the failing builds -O0, maybe?

--disable-optimize, so yes.
Fixes the problem for me. Thanks!
(In reply to Jed Davis from comment #11)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/a92aab848977 — fix
> for no-opt breakage, tested locally.

Have you filed a followup to get this fixed properly?
Aha, bug 1059038 is mentioned in a separate line in your checkin comment, which is why I didn't notice it the first time around. Sorry about that.
Depends on: 1059602
Flags: qe-verify-
This also breaks -flto build on Linux:

Executing: /var/tmp/gcc_test/usr/local/bin/c++ -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Werror=int-to-pointer-cast -Werror=type-limits -Wempty-body -Wsign-compare -Wno-invalid-offsetof -Wcast-align -flto=4 -ffunction-sections -fdata-sections -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe -DNDEBUG -DTRIMMED -O3 -fomit-frame-pointer -fPIC -shared -Wl,-z,defs -Wl,-h,libmozsandbox.so -o libmozsandbox.so /var/tmp/moz-build-dir/security/sandbox/linux/tmpZIKfIW.list -lpthread -Wl,--hash-style=gnu,--as-needed,--gc-sections,--icf=all,--icf-iterations=3 -Wl,-z,noexecstack -Wl,-z,text -Wl,--build-id -Wl,-rpath-link,/var/tmp/moz-build-dir/dist/bin -Wl,-rpath-link,/usr/lib -ldl -lrt
/var/tmp/moz-build-dir/security/sandbox/linux/tmpZIKfIW.list:
    INPUT("unused.o")
    INPUT("logging.o")
    INPUT("basicblock.o")
    INPUT("codegen.o")
    INPUT("die.o")
    INPUT("syscall.o")
    INPUT("Sandbox.o")
    INPUT("SandboxAssembler.o")
    INPUT("SandboxFilter.o")

/usr/bin/ld: error: /tmp/ccAwcilU.ltrans5.ltrans.o: requires dynamic R_X86_64_PC32 reloc against 'SyscallAsm' which may overflow at runtime; recompile with -fPIC
/usr/bin/ld: error: read-only segment has dynamic relocations
/tmp/ccAwcilU.ltrans5.ltrans.o:ccAwcilU.ltrans5.o:function sandbox::Die::SandboxDie(char const*, char const*, int) [clone .constprop.19]: error: undefined reference to 'SyscallAsm'
/tmp/ccAwcilU.ltrans5.ltrans.o:ccAwcilU.ltrans5.o:function sandbox::Die::SandboxDie(char const*, char const*, int) [clone .constprop.19]: error: undefined reference to 'SyscallAsm'
collect2: error: ld returned 1 exit status
/var/tmp/mozilla-central/config/rules.mk:829: recipe for target 'libmozsandbox.so' failed

An easy fix would be to disable -flto for syscall.cc:

diff --git a/security/sandbox/linux/moz.build b/security/sandbox/linux/moz.build
index 2cbbccf3bf48..c005d545defc 100644
--- a/security/sandbox/linux/moz.build
+++ b/security/sandbox/linux/moz.build
@@ -25,6 +25,9 @@ SOURCES += [
     'SandboxFilter.cpp',
 ]
 
+if '-flto' in CONFIG['OS_CXXFLAGS']:
+    SOURCES['../chromium/sandbox/linux/seccomp-bpf/syscall.cc'].flags += ['-fno-lto']
+
 DEFINES['NS_NO_XPCOM'] = True
 DISABLE_STL_WRAPPING = True
See Also: → 1062567
You need to log in before you can comment on or make changes to this bug.