Closed Bug 1503589 Opened Last year Closed Last year
Use the --enable-hardening configure flag for Linux builds
47 bytes, text/x-phabricator-request
|Details | Review|
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.
Reference: https://hg.mozilla.org/try/rev/cb0e6ff86d984a19bf88c630436c4b69cde50b47 https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb0e6ff86d984a19bf88c630436c4b69cde50b47 https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=cb0e6ff86d984a19bf88c630436c4b69cde50b47
Assignee: nobody → gpascutto
--enable-hardening build: https://hg.mozilla.org/try/rev/d507ede4ceaf2c8338589a16eb8a6229e810da5b https://treeherder.mozilla.org/#/jobs?repo=try&revision=d507ede4ceaf2c8338589a16eb8a6229e810da5b https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=d507ede4ceaf2c8338589a16eb8a6229e810da5b
Exception: Unknown platform(s) [linux64-st-an] specified for try Looks like a trychooser bug then?
2nd attempt: Reference: https://hg.mozilla.org/try/rev/057673fdf6638fc0ef376dd231b8f15df22e1436 https://treeherder.mozilla.org/#/jobs?repo=try&revision=057673fdf6638fc0ef376dd231b8f15df22e1436 https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=057673fdf6638fc0ef376dd231b8f15df22e1436 hardened: https://hg.mozilla.org/try/rev/daf6304c25fe237c3992e2e5834a43151caac5cc https://treeherder.mozilla.org/#/jobs?repo=try&revision=daf6304c25fe237c3992e2e5834a43151caac5cc https://treeherder.mozilla.org/perf.html#/comparechooser?newProject=try&newRevision=daf6304c25fe237c3992e2e5834a43151caac5cc
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 email@example.com: https://hg.mozilla.org/integration/autoland/rev/40d197664391 Enable strong stack protector by default. r=glandium
Backed out for causing windows MinGW build bustages. Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=busted&fromchange=40d19766439121cd6d231dbee225243d14939db0&tochange=235c2ef758a1e21f0520c5fed9cc3d4564f56c57&selectedJob=211925195 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=211925195&repo=autoland&lineNumber=4055 Backout link: https://hg.mozilla.org/integration/autoland/rev/235c2ef758a1e21f0520c5fed9cc3d4564f56c57
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
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?
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.
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.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/b8d320ccbfbc Enable strong stack protector by default. r=glandium
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."
You need to log in before you can comment on or make changes to this bug.