For bug 1277968 we're going to allow the firefox executable to run child processes, which means that any library dependencies in plugin-container's code for acting as a child process will be introduced into firefox, and we'd like to avoid that. The libmozsandbox dependencies are: * LinuxSandboxStarter, which can move back into libxul now that we won't need it in the executable after all * The call to SandboxEarlyInit, which only needs to be this early on B2G, which is no more (and didn't need library depdendencies pruned from the main executable like on desktop anyway) * The symbol-interposition wrappers for sigprocmask and pthread_sigmask, which need to read the signal number that libmozsandbox dynamically allocates to force all threads to start seccomp-bpf (if seccomp thread sync mode isn't available). Bug 1313218 will take care of that by moving them out of the executable.

Comment on attachment 8809656 [details] Bug 1313808 - Part 1: Move LinuxSandboxStarter back into libxul. https://reviewboard.mozilla.org/r/92192/#review92630 ::: ipc/app/moz.build:32 (Diff revision 1) > '/xpcom/base', > ] > > # We link GMPLoader into plugin-container on desktop so that its code is > # covered by the desktop DRM vendor's voucher. > -if CONFIG['OS_TARGET'] != 'Android': > +if CONFIG['OS_ARCH'] != 'Linux': I'm assuming OS_ARCH=='Linux' on Android? ::: ipc/contentproc/plugin-container.cpp:168 (Diff revision 1) > mozilla::SanitizeEnvironmentVariables(); > SetDllDirectoryW(L""); > } > #endif > -#if !defined(MOZ_WIDGET_ANDROID) && !defined(MOZ_WIDGET_GONK) && defined(MOZ_PLUGIN_CONTAINER) > +#if !defined(XP_LINUX) && defined(MOZ_PLUGIN_CONTAINER) > // On desktop, the GMPLoader lives in plugin-container, so that its s/On desktop/On Windows and MacOS/ ::: toolkit/xre/nsEmbedFunctions.cpp:320 (Diff revision 1) > #endif /* MOZ_CRASHREPORTER */ > > +#if defined (XP_LINUX) && defined(MOZ_GMP_SANDBOX) > +namespace { > +class LinuxSandboxStarter : public mozilla::gmp::SandboxStarter { > + LinuxSandboxStarter() { } Looks like the LinuxSandboxStarter has tab width 4, but the rest of this file has tab width 2.

Comment on attachment 8809657 [details] Bug 1313808 - Part 2: Move SandboxEarlyInit call into libxul. https://reviewboard.mozilla.org/r/92194/#review92800 ::: ipc/contentproc/plugin-container.cpp:32 (Diff revision 1) > -#include "mozilla/SandboxInfo.h" > -#endif > - > #ifdef MOZ_WIDGET_GONK > # include <sys/time.h> > # include <sys/resource.h> while you edit the file, you could remove the trailing whitespace here

Comment on attachment 8809656 [details] Bug 1313808 - Part 1: Move LinuxSandboxStarter back into libxul. https://reviewboard.mozilla.org/r/92192/#review92804 ok, when you fixed the 'issues' mentioned by :cpearce, it looks good to me

Comment on attachment 8809658 [details] Bug 1313808 - Part 3: GC the B2G-specific code in plugin-container.cpp. https://reviewboard.mozilla.org/r/92196/#review92828

Comment on attachment 8809656 [details] Bug 1313808 - Part 1: Move LinuxSandboxStarter back into libxul. https://reviewboard.mozilla.org/r/92192/#review92978 ::: toolkit/xre/moz.build:157 (Diff revision 1) > +if CONFIG['MOZ_SANDBOX'] and CONFIG['OS_ARCH'] == 'Linux': > + USE_LIBS += [ > + 'mozsandbox', > + ] Seems it would be simpler to just add a FINAL_LIBRARY = 'xul' to the moz.build file for mozsandbox.

Comment on attachment 8809657 [details] Bug 1313808 - Part 2: Move SandboxEarlyInit call into libxul. https://reviewboard.mozilla.org/r/92194/#review92982 ::: ipc/app/moz.build (Diff revision 1) > - # gcc lto likes to put the top level asm in syscall.cc in a different partition > - # from the function using it which breaks the build. Work around that by > - # forcing there to be only one partition. > - if '-flto' in CONFIG['OS_CXXFLAGS'] and not CONFIG['CLANG_CXX']: > - LDFLAGS += ['--param lto-partitions=1'] > - The comment suggests there's no reason this shouldn't still be needed. Although that might be required in a different moz.build now. It also seems part 1 and 2 ought to be folded together (does building with part 1 without part 2 produce a working build?)

(In reply to Mike Hommey [:glandium] from comment #8) > Seems it would be simpler to just add a FINAL_LIBRARY = 'xul' to the > moz.build file for mozsandbox. I thought that was only for static libraries; does it also work for shared libraries? (libmozsandbox needs to be linked separately from libxul to avoid symbol conflicts between ipc/chromium/src and security/sandbox/chromium, so it can't be part of libxul.so; we also have this issue on Windows.)

Oh, sorry, I thought it was a static library.

OTOH, why is it not a static library if only libxul uses it?

Comment on attachment 8809657 [details] Bug 1313808 - Part 2: Move SandboxEarlyInit call into libxul. https://reviewboard.mozilla.org/r/92194/#review93284 ::: ipc/app/moz.build (Diff revision 1) > - # gcc lto likes to put the top level asm in syscall.cc in a different partition > - # from the function using it which breaks the build. Work around that by > - # forcing there to be only one partition. > - if '-flto' in CONFIG['OS_CXXFLAGS'] and not CONFIG['CLANG_CXX']: > - LDFLAGS += ['--param lto-partitions=1'] > - It's needed only for the linkage unit that contains the Chromium sandboxing code, so this actually hasn't been needed here since `mozsandbox` stopped being statically linked into `plugin-container` (bug 1268733). There's already an instance of this in `security/sandbox/linux/moz.build` for `libmozsandbox.so`, which is the only build product that needs it now. And I noticed while I was writing that that there's another copy of this in `ipc/app/pie`, which can also go away (especially because `MOZ_SANDBOX` has never been true for Android Fennec). ::: ipc/contentproc/plugin-container.cpp:32 (Diff revision 1) > -#include "mozilla/SandboxInfo.h" > -#endif > - > #ifdef MOZ_WIDGET_GONK > # include <sys/time.h> > # include <sys/resource.h> This entire ifdef block gets deleted in the last patch.

(In reply to Mike Hommey [:glandium] from comment #9) > It also seems part 1 and 2 ought to be folded together (does building with > part 1 without part 2 produce a working build?) It does; I tested it locally.

Comment on attachment 8809657 [details] Bug 1313808 - Part 2: Move SandboxEarlyInit call into libxul. https://reviewboard.mozilla.org/r/92194/#review94850

Pushed by jedavis@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/40c8ee5e5f1d Part 1: Move LinuxSandboxStarter back into libxul. r=cpearce,glandium,tedd https://hg.mozilla.org/integration/autoland/rev/b9993235ff99 Part 2: Move SandboxEarlyInit call into libxul. r=glandium,tedd https://hg.mozilla.org/integration/autoland/rev/f591a4672390 Part 3: GC the B2G-specific code in plugin-container.cpp. r=billm

https://hg.mozilla.org/mozilla-central/rev/40c8ee5e5f1d https://hg.mozilla.org/mozilla-central/rev/b9993235ff99 https://hg.mozilla.org/mozilla-central/rev/f591a4672390

Let it ride the train with 53