Bug 233707 - www/firefox: fails to build with -fstack-protector-{strong,all} + -Wl,-z,nocopyreloc
Summary: www/firefox: fails to build with -fstack-protector-{strong,all} + -Wl,-z,noco...
Status: New
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-toolchain mailing list
Keywords: regression
Depends on:
Blocks: 214864
  Show dependency treegraph
Reported: 2018-12-02 05:28 UTC by Jan Beich
Modified: 2019-02-06 03:42 UTC (History)
7 users (show)

See Also:
bugzilla: maintainer-feedback? (gecko)

quick fix (460 bytes, patch)
2019-01-29 16:01 UTC, Tijl Coosemans
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer 2018-12-02 05:28:45 UTC
Firefox 63 enabled -Wl,-z,nocopyreloc which broke build with non-default SSP_C?FLAGS but Firefox 65 enable -fstack-protector-strong which broke even default builds.

$ cc -v
FreeBSD clang version 7.0.1 (branches/release_70 346007) (based on LLVM 7.0.1)
Target: x86_64-unknown-freebsd13.0
Thread model: posix
InstalledDir: /usr/bin

$ cat >a.c
#include <time.h>

int main() {
  struct timespec ts;
  clock_gettime(CLOCK_MONOTONIC, &ts);
  return 0;

$ cc -fstack-protector-strong -Wl,-z,nocopyreloc a.c
ld: error: unresolvable relocation R_X86_64_PC32 against symbol '__stack_chk_guard'; recompile with -fPIC or remove '-z nocopyreloc'
>>> defined in /lib/libc.so.7
>>> referenced by a.c
>>>               /tmp/a-b6cdf3.o:(main)
cc: error: linker command failed with exit code 1 (use -v to see invocation)
Comment 1 Dimitry Andric freebsd_committer 2018-12-04 19:08:16 UTC
Indeed.  Output from ld.lld 7.0.1 rc2:

$ ld.lld --eh-frame-hdr -dynamic-linker /libexec/ld-elf.so.1 --hash-style=both --enable-new-dtags -o bug233707 /usr/lib/crt1.o /usr/lib/crti.o /usr/lib/crtbegin.o -L/usr/lib -z nocopyreloc bug233707.o -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed /usr/lib/crtend.o /usr/lib/crtn.o
ld.lld: error: unresolvable relocation R_X86_64_PC32 against symbol '__stack_chk_guard'; recompile with -fPIC or remove '-z nocopyreloc'
>>> defined in /lib/libc.so.7
>>> referenced by bug233707.c
>>>               bug233707.o:(main)

Output from ld.bfd 2.17.50 (in base):

$ /usr/bin/ld.bfd --eh-frame-hdr -dynamic-linker /libexec/ld-elf.so.1 --hash-style=both --enable-new-dtags -o bug233707 /usr/lib/crt1.o /usr/lib/crti.o /usr/lib/crtbegin.o -L/usr/lib -z nocopyreloc bug233707.o -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed /usr/lib/crtend.o /usr/lib/crtn.o

Output from ld.bfd 2.30 (from ports):

$ ld.bfd --eh-frame-hdr -dynamic-linker /libexec/ld-elf.so.1 --hash-style=both --enable-new-dtags -o bug233707 /usr/lib/crt1.o /usr/lib/crti.o /usr/lib/crtbegin.o -L/usr/lib -z nocopyreloc bug233707.o -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed /usr/lib/crtend.o /usr/lib/crtn.o
ld.bfd: bug233707.o: relocation R_X86_64_PC32 against symbol `__stack_chk_guard@@FBSD_1.0' can not be used when making a PDE object; recompile with -fPIC
ld.bfd: final link failed: Bad value

It's interesting how ld.bfd is talking about a "PDE object" here.  I guess that means a Position Dependent Executable...

Maybe a workaround is to compile firefox with -fPIE? :)
Comment 2 Shawn Webb 2018-12-05 02:12:19 UTC
Compilation with -fstack-protector-all is working fine in HardenedBSD: https://github.com/HardenedBSD/hardenedbsd-ports/commit/03fb5f43e1322cb4c49e9834643b9c1361a04930

I haven't tried setting -Wl,-z,nocopyreloc though. I will soon.
Comment 3 Tijl Coosemans freebsd_committer 2018-12-05 10:52:32 UTC
It seems that on Linux __stack_check_guard is no longer used.  They use a TLS variable (%fs:40 on amd64).
Comment 4 Jan Beich freebsd_committer 2018-12-05 11:06:53 UTC
Default SSP_CFLAGS appear to cancel -fstack-protector-strong, so the port builds fine: https://ptpb.pw/G4f6

Upstream build isn't so fortunate:

$ pkg install python27
$ hash git 2>/dev/null || pkg install mercurial
$ hg clone https://hg.mozilla.org/mozilla-unified firefox ||
  git clone https://github.com/mozilla/gecko-dev firefox
$ cd firefox
$ hg update central || git checkout origin/master
$ ./mach bootstrap # select Firefox for Desktop
$ ./mach build
checking for clock_gettime(CLOCK_MONOTONIC)... no
The error occurred while processing the following file:
A moz.build file called the error() function.
The error it encountered is:
    No TimeStamp implementation on this platform.  Build will not succeed

(In reply to Dimitry Andric from comment #1)
> Maybe a workaround is to compile firefox with -fPIE? :)

ld: error: relocation R_X86_64_PC32 cannot be used against symbol SEC_PrintableStringTemplate; recompile with -fPIC
>>> defined in ../util_nssutil/secasn1d.o
>>> referenced by secasn1d.c
>>>               ../util_nssutil/secasn1d.o:(SEC_ASN1DecoderUpdate_Util)
Comment 5 Shawn Webb 2018-12-05 15:36:17 UTC
I think the reason why FreeBSD is exhibiting issues but not HardenedBSD is because we enforce -fPIC for all libraries, both in base and ports. We need to do so to properly build applications as PIEs. Forcing PIC also helps protect against a compiler-level security vulnerability[1].

[1]: http://www.cse.psu.edu/~trj1/papers/ndss17.pdf
Comment 6 Jan Beich freebsd_committer 2018-12-30 04:16:59 UTC
I've filed an upstream bug to get more feedback.

(In reply to Shawn Webb from comment #5)
OpenBSD is also unaffected.

Comment 7 Shawn Webb 2018-12-30 14:12:56 UTC
(In reply to Jan Beich from comment #6)
> I've filed an upstream bug to get more feedback.

I doubt this is a bug in upstream. Every major operating system in which Mozilla supports supports ASLR, with the sole exception of FreeBSD. The problem is that FreeBSD isn't compiling certain libraries with -fPIC. Once FreeBSD gains some form of address space randomization, whether it be ASR or ASLR, FreeBSD will also need to update base and ports to compile libraries with -fPIC, which HardenedBSD has already done (and, it appears, OpenBSD, too, but I haven't verified that). Granted, the `-fPIC`-ization could happen before the ASR[1] patch lands (and likely would be good preparation for it).

I think Mozilla is in the right here because they're applying security hardening measures. There'd be two ways to fix this: 1) apply fewer security hardening measures in the browser; 2) apply -fPIC where appropriate. Option 2 is the more attractive option. Granted, browsers are extremely complex applications that are nearly impossible to properly secure, especially given that they execute arbitrary remote code locally.

[1]: https://reviews.freebsd.org/D5603
Comment 8 Tijl Coosemans freebsd_committer 2018-12-30 15:04:26 UTC
(In reply to Shawn Webb from comment #7)
Firefox is already compiled with -fPIE on FreeBSD and this works fine even with -Wl,-z,nocopyreloc and -fstack-protector-strong.  The problem is only in the configure test for clock_gettime which is *not* compiled with -fPIE but *is* compiled with -Wl,-z,nocopyreloc and -fstack-protector-strong which enables stack protection in this test program while -fstack-protector is a no-op.  Stack protection uses a variable named __stack_chk_guard which is defined in libc on FreeBSD.  When compiled without -fPIE, variables in dynamic libraries accessed by the executable are copied to the executable's data segment so it can be accessed directly (without GOT lookup).  This is called a copy-relocation, which is why compiling with -Wl,-z,nocopyreloc gives an error.  So either Firefox should compile configure tests with -fPIE or FreeBSD should do what Linux does and make __stack_chk_guard a thread local variable (or both).
Comment 9 Shawn Webb 2018-12-30 15:17:01 UTC
(In reply to Tijl Coosemans from comment #8)
Almost. Compiling with -fPIC will solve the issue. Take a look at the article I linked to in comment #5. HardenedBSD does this and is why HardenedBSD can continue using SSP-all with www/firefox without issues. In this case, -fPIE isn't needed, but -fPIC is.
Comment 10 Shawn Webb 2018-12-30 15:20:36 UTC
(In reply to Shawn Webb from comment #9)
But, perhaps, it might be working in HardenedBSD due to our forcing CFLAGS+=-fPIE on applications. Perhaps you're right. Let me do some extra digging and I'll report back.
Comment 11 Tijl Coosemans freebsd_committer 2018-12-30 15:48:19 UTC
(In reply to Shawn Webb from comment #9)
The paper is about copy-relocations of read-only variables.  There are potential security problems with that because the copy is writeable.  The copy has to be initialised at run-time with the value from the library.  That doesn't apply to __stack_chk_guard because it's already writeable.
Comment 12 Shawn Webb 2018-12-30 16:11:03 UTC
(In reply to Tijl Coosemans from comment #11)
You're right, adding -fPIE to CFLAGS does resolve the issue. Can FreeBSD do that in order to support a stronger SSP with Firefox?
Comment 13 Shawn Webb 2018-12-30 16:17:51 UTC
(In reply to Shawn Webb from comment #12)
I did verify that adding -fPIC does resolve the issue, too. So, -fPIC and -fPIE can be used interchangeably in this case.
Comment 14 Tijl Coosemans freebsd_committer 2018-12-30 17:00:33 UTC
(In reply to Shawn Webb from comment #13)
-fPIE == -fPIC + some optimisations that only apply to executables.  Executables can access their own global variables and functions directly.  Libraries cannot (unless the variable is declared with protected visibility) because a symbol with the same name can be defined in the executable or another library and the symbol can be resolved to that one.  So you can compile executables with -fPIC but you should not compile shared libraries with -fPIE.
Comment 15 rozhuk.im 2019-01-23 21:36:29 UTC
Why not add to Makefile:

and close this?
Comment 16 Tijl Coosemans freebsd_committer 2019-01-24 09:54:32 UTC
(In reply to rozhuk.im from comment #15)
Because then every file gets compiled with -fPIC.
Comment 17 Tijl Coosemans freebsd_committer 2019-01-29 16:01:59 UTC
Created attachment 201510 [details]
quick fix

With this change to Mk/bsd.gecko.mk the clock_gettime(CLOCK_MONOTONIC) check is skipped.