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: Closed FIXED
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
URL:
Keywords: regression
Depends on:
Blocks: 214864 237273
  Show dependency treegraph
 
Reported: 2018-12-02 05:28 UTC by Jan Beich
Modified: 2019-09-27 04:37 UTC (History)
7 users (show)

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


Attachments
quick fix (460 bytes, patch)
2019-01-29 16:01 UTC, Tijl Coosemans
no flags Details | Diff
add linker flag to tests (2.35 KB, patch)
2019-03-07 08:16 UTC, rozhuk.im
no flags Details | Diff
log (235.85 KB, text/plain)
2019-03-15 07:34 UTC, rozhuk.im
no flags Details
patch (2.35 KB, patch)
2019-03-15 21:08 UTC, rozhuk.im
rozhuk.im: maintainer-approval+
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
<zilch>

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
[...]
mozbuild.frontend.reader.BuildReaderError:
==============================
FATAL ERROR PROCESSING MOZBUILD FILE
==============================
The error occurred while processing the following file:
    /path/to/mozilla-central/mozglue/misc/moz.build
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? :)

$ CFLAGS=-fPIE CXXFLAGS=$CFLAGS ./mach build
[...]
security/nss/lib/util/libnssutil3.so
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.

http://buildbot.rhaalovely.net/nine/#/builders/3/builds/352
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:
CFLAGS+=	-fPIC
CPPFLAGS+=	-fPIC

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.
Comment 18 commit-hook freebsd_committer 2019-03-06 04:08:16 UTC
A commit references this bug:

Author: jbeich
Date: Wed Mar  6 04:07:50 UTC 2019
New revision: 494772
URL: https://svnweb.freebsd.org/changeset/ports/494772

Log:
  www/firefox: unbreak on non-x86

  checking for clock_gettime(CLOCK_MONOTONIC)... no
  [...]
  The error occurred while processing the following file:

      /wrkdirs/usr/ports/www/firefox/work/firefox-65.0.2/mozglue/misc/moz.build

  A moz.build file called the error() function.

  The error it encountered is:

      No TimeStamp implementation on this platform.  Build will not succeed

  Correct the error condition and try again.

  PR:		233707
  Reported by:	bob prohaska, pkg-fallout
  Submitted by:	tijl

Changes:
  head/Mk/bsd.gecko.mk
Comment 19 rozhuk.im 2019-03-06 09:19:29 UTC
Does not works for me.

This work:
MOZ_EXPORT+=	${CONFIGURE_ENV} \
				RUSTFLAGS="${RUSTFLAGS}" \
				PERL="${PERL}" \
				EXPAND_LIBS_LIST_STYLE="filelist" \
				ac_cv_struct_tm_zone_tm_gmtoff="yes" \
				ac_cv_clock_monotonic="yes"
Comment 20 rozhuk.im 2019-03-06 11:35:14 UTC
(In reply to rozhuk.im from comment #19)
Works for config, fail on build, can not find -lyes, with EXPAND_LIBS_LIST_STYLE="list"
Comment 21 rozhuk.im 2019-03-07 08:16:53 UTC
Created attachment 202675 [details]
add linker flag to tests

I made a proper patch, now no errors on configure and build, it does not need last change that commitetd into ports tree - 494772.
Comment 22 Tijl Coosemans freebsd_committer 2019-03-07 09:14:56 UTC
(In reply to rozhuk.im from comment #20)
Can you show the actual error?  I think it's looking for -lyes because you have ac_cv_clock_monotonic="yes" instead of ac_cv_clock_monotonic=
Comment 23 rozhuk.im 2019-03-07 10:14:33 UTC
(In reply to Tijl Coosemans from comment #22)
I do not save it, IMHO mine last patch is better, because it does not disable clock_gettime support and fix only tests that fail without -fPIC.
Probably ac_cv_clock_monotonic="yes" come to -lyes.
Comment 24 Tijl Coosemans freebsd_committer 2019-03-07 10:30:26 UTC
(In reply to rozhuk.im from comment #23)
The change committed to the ports tree says that no libraries are needed to use clock_gettime.  It does not disable clock_gettime.  And it's not -fPIC that is missing but -fPIE.  I always build with -fstack-protector-strong and I'm not seeing any build error.  I suspect the error you had was caused by local changes.
Comment 25 rozhuk.im 2019-03-07 10:38:37 UTC
(In reply to Tijl Coosemans from comment #24)
Try -fstack-protector-all.

Mine make.conf (without comments)
CFLAGS+=-O2 -DSTRIP_FBSDID -pipe
CXXFLAGS+=-DSTRIP_FBSDID
COPTFLAGS+=-O2 -funroll-loops -DSTRIP_FBSDID -pipe
SSP_CFLAGS=-fstack-protector-all
WITH_SSP=yes
WITH_SSP_PORTS=yes
CFLAGS+=-D_FORTIFY_SOURCE=2
CXXFLAGS+=-D_FORTIFY_SOURCE=2
COPTFLAGS+=-D_FORTIFY_SOURCE=2
CFLAGS+=-mretpoline
CXXFLAGS+=-mretpoline
COPTFLAGS+=-mretpoline
Comment 26 Tijl Coosemans freebsd_committer 2019-03-08 11:48:41 UTC
(In reply to rozhuk.im from comment #25)
Current port builds fine here with -fstack-protector-all.
Comment 27 rozhuk.im 2019-03-11 12:09:32 UTC
(In reply to Tijl Coosemans from comment #26)
I do mine tests on 12.0 stable, less than 1 month updated.
I see at least two error message about __stack_chk_guard in config.log file, that is why I add -fPIC in few places.
IMHO adding -fPIC more correct way to fix this.
Comment 28 Tijl Coosemans freebsd_committer 2019-03-11 12:55:49 UTC
Can you attach your config.log to this bug?
Comment 29 rozhuk.im 2019-03-15 07:34:59 UTC
Created attachment 202878 [details]
log

This is for latest firefox-66.0_2,1, without mine pathes.
Comment 30 Tijl Coosemans freebsd_committer 2019-03-15 15:45:21 UTC
(In reply to rozhuk.im from comment #29)
Ok, that does look like your patch is necessary, but please change it to use -fPIE instead of -fPIC.  Does the patch address the problem with clock_gettime as well or only these additional cases?
Comment 31 rozhuk.im 2019-03-15 21:07:16 UTC
Yes, it fix clock_gettime for me too.
Comment 32 rozhuk.im 2019-03-15 21:08:10 UTC
Created attachment 202889 [details]
patch
Comment 33 Jan Beich freebsd_committer 2019-04-08 07:50:48 UTC
Upstream has a similar fix.
Comment 34 commit-hook freebsd_committer 2019-04-08 11:51:37 UTC
A commit references this bug:

Author: jbeich
Date: Mon Apr  8 11:50:52 UTC 2019
New revision: 498362
URL: https://svnweb.freebsd.org/changeset/ports/498362

Log:
  www/firefox: switch to upstream fix for non-x86

  PR:		233707

Changes:
  head/Mk/bsd.gecko.mk
  head/www/firefox/files/patch-bug1513605
Comment 35 commit-hook freebsd_committer 2019-04-08 12:40:38 UTC
A commit references this bug:

Author: jbeich
Date: Mon Apr  8 12:40:02 UTC 2019
New revision: 498370
URL: https://svnweb.freebsd.org/changeset/ports/498370

Log:
  MFH: r498362

  www/firefox: switch to upstream fix for non-x86

  PR:		233707
  Approved by:	ports-secteam blanket

Changes:
_U  branches/2019Q2/
  branches/2019Q2/Mk/bsd.gecko.mk
  branches/2019Q2/www/firefox/files/patch-bug1513605
Comment 36 Jan Beich freebsd_committer 2019-04-08 14:55:55 UTC
rozhuk.im, can you try again without your patch?
Comment 37 rozhuk.im 2019-04-11 23:30:39 UTC
(In reply to Jan Beich from comment #36)

Now OK without any patches that not in tree.