Closed Bug 1503589 Opened Last year Closed Last year

Use the --enable-hardening configure flag for Linux builds

Categories

(Core :: Security, enhancement)

x86_64
Linux
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
relnote-firefox --- 65+
firefox65 --- fixed

People

(Reporter: gcp, Assigned: gcp)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file, 1 obsolete file)

This bug is to cover to turning on the --enable-hardening flag on Linux builds. 

Turning this on is going to include the changes to set the flag and also performance testing to determine performance impacts.

Our understanding is that Ubuntu is shipping this configuration for their builds already.
Blocks: 1359928
Exception: Unknown platform(s) [linux64-st-an] specified for try

Looks like a trychooser bug then?
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=057673fdf6638fc0ef376dd231b8f15df22e1436&newProject=try&newRevision=daf6304c25fe237c3992e2e5834a43151caac5cc&framework=1&showOnlyConfident=1

Pref regression is typically <1%, except for

perf_reftest_singletons pgo e10s stylo   2.6%
a11yr opt e10s stylo                     4.5%

We said before we're willing to take a 2-3% performance hit for this, so this looks good. Digging into the a11yr result might be useful as it's possible some snipped of code suffers from the stack checks for some reason.
Context for reviewers: many of the blockers for enabling hardening (and things like PIE) have been eliminated in the last few years. There was a request to not enable hardening (and any perf hit) before the first Quantum release. This has passed now, so we did a new Talos run. The regressions there are very small, and probably do not outweigh the security benefit.
Much of the original discussion is in: https://bugzilla.mozilla.org/show_bug.cgi?id=620058
Attachment #9022922 - Attachment is obsolete: true
Pushed by gpascutto@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/40d197664391
Enable strong stack protector by default. r=glandium
The build that failed doesn't even define -fstack-protector-strong. What's going on?

[task 2018-11-15T10:25:57.194Z] 10:25:57     INFO -  /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/x86_64-w64-mingw32-clang++ -mwindows -o StoreBuffer.o -c  -DDEBUG=1 -DENABLE_BINARYDATA -DENABLE_WASM_BULKMEM_OPS -DENABLE_WASM_GC -DENABLE_WASM_GENERALIZED_TABLES -DWASM_PRIVATE_REFTYPES -DWASM_HUGE_MEMORY -DJS_CACHEIR_SPEW -DENABLE_SHARED_ARRAY_BUFFER -DENABLE_WASM_THREAD_OPS -DJS_HAS_CTYPES -DFFI_BUILDING -DEXPORT_JS_API -DMOZ_HAS_MOZGLUE -I/builds/worker/workspace/build/src/js/src/gc -I/builds/worker/workspace/build/src/obj-firefox/js/src/gc -I/builds/worker/workspace/build/src/obj-firefox/js/src -I/builds/worker/workspace/build/src/js/src -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/js/src/js-confdefs.h -Qunused-arguments -Qunused-arguments -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=return-std-move -Wno-error=atomic-alignment -Wno-unknown-pragmas -Wno-unused-function -Wno-conversion-null -Wno-switch -Wno-enum-compare -Wno-gnu-zero-variadic-macro-arguments -Wno-noexcept-type -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-sized-deallocation -fms-extensions -mms-bitfields -Wno-incompatible-ms-struct -fno-rtti -ffunction-sections -fdata-sections -Wa,-mbig-obj -fno-exceptions -fno-math-errno -pipe -g -gcodeview -fno-omit-frame-pointer -funwind-tables -fno-strict-aliasing -Werror=format -Wno-shadow  -MD -MP -MF .deps/StoreBuffer.o.pp   /builds/worker/workspace/build/src/js/src/gc/StoreBuffer.cpp
Flags: needinfo?(gpascutto)
tjr pointed out that some configure checks, including the one for _thread, give a different result. I noticed it even changes whether truncate64 support is detected, which is just very odd.
The problem seems to be that in our toolchain, we have lib/libssp.a and lib/libssp_nonshared.a but we look for libraries in x86_64-w64-mingw32/lib/

Symlinking these two libraries into x86_64-w64-mingw32/lib/ makes the configure tests pass, which should resolve the build error.  I just want to double check that this makes sense and is safe.  These libraries are also in lib32/ and symlinking those versions _also_ works. Which version should be use for x64? The same files are present in the same place for x86; which version should we use there?
Flags: needinfo?(jacek)
Probably we shouldn't use any of them. Those are host libraries, we'd need something for cross target. Here is how llvm-mingw toolchain builds it:
https://github.com/mstorsjo/llvm-mingw/blob/master/build-libssp.sh
We could do the same in our toolchain build script (we don't need .dll version, just static library, so it's a bit simpler than tat) or even use llvm-mingw script for that.


Another way to look at it is that this bug is about Linux build. mingw builds are made on Linux, but those are Windows builds. Thus it's not obvious to me that enabling it on mingw clang is intentional as far as this bug is considered.
Flags: needinfo?(jacek)
gcp: I'm not going to be able to dig into that in the short term; we should disable stack protector for mingw-clang, and file a followup bug blocking 1330608.
Blocks: 1511073
Pushed by gpascutto@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b8d320ccbfbc
Enable strong stack protector by default. r=glandium
https://hg.mozilla.org/mozilla-central/rev/b8d320ccbfbc
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Depends on: 1512162
Relnote (together with bug 1492917):

"Firefox on Linux and macOS now enables stronger stack smashing protection by default."
Amended release note:

"Firefox on Linux, Android and macOS now enables stronger stack smashing protection by default."
relnote-firefox: --- → ?
Depends on: 1516830
Depends on: 1533133
You need to log in before you can comment on or make changes to this bug.