1041886 - Separate Linux sandbox code into its own shared library

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

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.)

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: bug1041886-p0-sandbox-log-header-hg1.diff

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.

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: bug1041886-p1-sandbox-lib-hg1.diff

…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.

Comment 3

[Mike Hommey [:glandium]]

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

Comment 4

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b3dcb5b33f78
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f9d0821e08c

Comment 5

[Matthew Gregan [:kinetik]]

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

Comment 6

[Nicholas Nethercote [inactive]]

I also hit this link error on my Linux box.

Comment 7

[Mike Hommey [:glandium]]

Sounds like a visibility issue.

Comment 8

[Mike Hommey [:glandium]]

I can't reproduce, fwiw. What's your version of gold? (gold --version)

Comment 9

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

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.

Comment 10

[Matthew Gregan [:kinetik]]

(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.

Comment 11

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a92aab848977 — fix for no-opt breakage, tested locally.

Comment 12

[Nicholas Nethercote [inactive]]

Fixes the problem for me. Thanks!

Comment 13

[[e-mail]]

(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?

Comment 14

[[e-mail]]

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.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b3dcb5b33f78
https://hg.mozilla.org/mozilla-central/rev/2f9d0821e08c
https://hg.mozilla.org/mozilla-central/rev/a92aab848977

Comment 16

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/517090126627

Comment 17

[Octoploid]

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

Comment 18

[Octoploid]

See: https://gcc.gnu.org/wiki/LinkTimeOptimizationFAQ#Symbol_usage_from_assembly_language
for further info.