1079662 - Enable PIE on Linux

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

Plain enabling leads to bug 1076892. Something needs to be done about that before PIE can be enabled on mozilla.org builds.

Comment 1

[Johannes Pfrang [:johnp]]

Fwiw, in pursue of the "Harden All packages"-objective[1], Fedora is now building Firefox with PIE:

http://pkgs.fedoraproject.org/cgit/firefox.git/commit/?h=f22&id=ded1820a4f7f445b440a40a0e584bf3038307066

Their experience may be helpful. Also the PIE-enabled user base may push libmagic or file manager developers to provide a fix for bug 1076892.

[1] https://fedoraproject.org/wiki/Changes/Harden_All_Packages

Comment 2

[Mike Hommey [:glandium]]

Fedora doesn't have our problem that people will be starting Firefox from a file manager. I've been building Iceweasel PIE in Debian for a while.

Comment 3

[Mike Hommey [:glandium]]

(In reply to Johannes Pfrang [:johnp] from comment #1)
> Fwiw, in pursue of the "Harden All packages"-objective[1], Fedora is now
> building Firefox with PIE:
> 
> http://pkgs.fedoraproject.org/cgit/firefox.git/commit/
> ?h=f22&id=ded1820a4f7f445b440a40a0e584bf3038307066

Also, that change is wrong, it's passing -pie where it shouldn't. Use the --enable-pie configure option instead.

Comment 4

[Mike Hommey [:glandium]]

Attachment: Bug 1079662 - Use firefox-bin for updater tests on Linux. r?rstrong

This sort of reverts the changes from bug 771940, for Linux only,
because we're going to turn the firefox binary into a wrapper that
executes firefox-bin, and since the updater tests only copy the former,
they fail when trying to start firefox. Making them instead use
firefox-bin directly works around the issue. This matches what we were
doing back when firefox was a shell script that executed firefox-bin.

Comment 5

[Mike Hommey [:glandium]]

Attachment: Bug 1079662 - Always pass -rdynamic when linking with sanitizers. r?froydnj

When building executables as PIE, and because we use -Bsymbolic, which
symbols are exported from an executable varies based on the libraries
it's directly linked against, to fulfil their symbol needs. So when a
library depends on e.g. ASAN runtime symbols, and the linker finds that,
it will keep those ASAN symbols in the executable for the library. And
drop the other, unused symbols.

But when the executable then dlopen()s a library (e.g. shlibsign loading
libfreebl) that uses another set of ASAN symbols, including symbols that
none of the direct dependencies of the executable need, dlopen() fails
because of the missing symbols.

It's not currently an apparent problem because we don't enable PIE, and
we build Gecko executables with -rdynamic already (for mozjemalloc). But
we don't build non-Gecko executables this way (like shlibsign).

Depends on D5107

Comment 6

[Mike Hommey [:glandium]]

Attachment: Bug 1079662 - Always enable PIE. r?froydnj

Last attempt, a few years ago, blatantly failed because nautilus (the
GNOME file manager) can't start PIE executables, which look like shared
libraries, and that it thus considers not being executables.

Downstreams don't actually have the problem, because users won't be
launching Firefox from a file manager, but for mozilla.org builds, it is
a problem because users would download, then extract, and then likely
try to run the Firefox executable from a file manager.

So for mozilla.org builds, we still need to find a way around the
nautilus problem.

A .desktop file could be a solution, but .desktop files have not
actually been designed for this use case, which leads to:
- having to use an awful one-liner shell wrapper to derive the path
  to the executable from that of the .desktop file,
- not even being able to associate an icon,
- the .desktop file not being copiable to a location where .desktop
  files would normally go, because it would then fail to find the
  executable.

Another possibility is to go back to using a shell wrapper, but that's
not entirely appealing.

What we chose here is similar, where we have a small `firefox` wrapper
that launches the real `firefox-bin` (which is still leftover from those
old times where we had a shell wrapper, for reasons).

The small `firefox` wrapper is a minimalist C executable that just
finds the path to the `firefox-bin` executable and executes it with the
same args it was called with. The wrapper is only enabled when the
MOZ_NO_PIE_COMPAT environment variable is set, which we only take into
account on Linux. The variable is only really meant to be used for
mozilla.org builds, for the nautilus problem. Downstreams will just pick
the default, which is changed to build PIE.

On other platforms, PIE was already enabled by default, so we just
remove the --enable-pie configure flag.

Depends on D5108

Comment 7

[Robert Strong (they/them - no direct email)]

Comment on attachment 9006765 [details]
Bug 1079662 - Use firefox-bin for updater tests on Linux. r?rstrong

Robert Strong [:rstrong] (use needinfo to contact me) has approved the revision.

Comment 8

[Nathan Froyd [:froydnj]]

Comment on attachment 9006766 [details]
Bug 1079662 - Always pass -rdynamic when linking with sanitizers. r?froydnj

Nathan Froyd [:froydnj] has approved the revision.

Comment 9

[Nathan Froyd [:froydnj]]

Comment on attachment 9006767 [details]
Bug 1079662 - Always enable PIE. r?froydnj

Nathan Froyd [:froydnj] has approved the revision.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1cfac7a05877
Use firefox-bin for updater tests on Linux. r=rstrong
https://hg.mozilla.org/integration/mozilla-inbound/rev/877c851a1b66
Always pass -rdynamic when linking with sanitizers. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/3cbbfc5127e4
Always enable PIE. r=froydnj

Comment 11

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/1cfac7a05877
https://hg.mozilla.org/mozilla-central/rev/877c851a1b66
https://hg.mozilla.org/mozilla-central/rev/3cbbfc5127e4

Comment 13

[Bryan Quigley]

Thanks Mike!  Just FYI - Nautilus dropped the ability to launch any apps (IMU*) - https://gitlab.gnome.org/GNOME/nautilus/commit/3a22ed5b8e3bbc1c59ff3069ee79755168754916

I don't think it makes sense to hold up Firefox security because of Nautilus' decisions done for their security.  The workaround you created will work for all current users until that new version of Nautilus lands. (and for many LTS versions until EOL)

*I couldn't actually get a test of Nautilus 3.30 going.

Comment 14

[Christian Holler (:decoder)]

This caused a local startup crash for a fuzzing+asan+coverage build:

The first bad revision is:
changeset:   435295:3cbbfc5127e4
user:        Mike Hommey <[e-mail]>
date:        Thu Sep 06 13:27:49 2018 +0900
summary:     Bug 1079662 - Always enable PIE. r=froydnj

(crash has no symbols for me, the trace isn't really helpful).

I am debugging right now which of the factors is required for the crash. It does not seem to affect the same build configuration that we have in TC automation.

Comment 15

[Christian Holler (:decoder)]

(In reply to Christian Holler (:decoder) from comment #14)
>
> I am debugging right now which of the factors is required for the crash. It
> does not seem to affect the same build configuration that we have in TC
> automation.

Small update here: The crash occurs with a quite regular mozconfig when built locally on Ubuntu 18.04:


export CC="clang-7"
export CXX="clang++-7"
ac_add_options --enable-address-sanitizer
ac_add_options --enable-fuzzing
ac_add_options --disable-jemalloc
ac_add_options --disable-crashreporter
ac_add_options --enable-valgrind
ac_add_options --disable-install-strip
ac_add_options --enable-optimize="-O2 -gline-tables-only"
ac_add_options --disable-debug


Crash:

$ FUZZER=AV1Decode MOZ_RUN_GTEST=1 LIBFUZZER=1 dist/bin/firefox
Running Fuzzer tests...
AddressSanitizer:DEADLYSIGNAL
=================================================================
==9025==ERROR: AddressSanitizer: SEGV on unknown address 0x5640ca064000 (pc 0x5640ca064047 bp 0x7ffc6abefa90 sp 0x7ffc6abee3b8 T0)
==9025==The signal is caused by a WRITE memory access.
    #0 0x5640ca064046 in __sched_cpucount (dist/bin/firefox+0x46)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (dist/bin/firefox+0x46) in __sched_cpucount
==9025==ABORTING


This is a blocker for some of our fuzzing efforts so it would be good to fix this quickly or backout the change that caused this until we know the root cause.

Comment 16

[Christian Holler (:decoder)]

I filed bug 1490845 for the regression in comment 14.

Comment 18

[ISHIKAWA, Chiaki]

There was a ASAN bug report regarding its use under certain linux kernel, in the following.
Basically, we can't use PIE binary.
Maybe this is it?
https://github.com/google/sanitizers/issues/856#issuecomment-327657374
https://github.com/google/sanitizers/issues/856