1511073 - Enable stack protector on mingw-clang builds

Status: RESOLVED FIXED

(Core :: Security, enhancement)

Description

[Gian-Carlo Pascutto [:gcp]]

mingw-clang has an exclusion due to this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1503589#c18

We should remove the exclusion when the toolchain is fixed.

Comment 1

[Georg Koppen]

Quoting from bug 1503589:

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.

So, I tried a bunch of things (including building the libssp libs as llvm-mingw does it). But in all cases it turns out that a) we need the .dll version as all the binaries seem to get linked against it. And b) the resulting bundles crash both on 64bit and 32bit Windows systems.

On the Firefox side I only removed the mingw-w64-clang exclusion for stack protection support.

I won't have time to look at it closer before we switch to ESR 68 at the end of October.

Comment 2

[Martin Storsjö]

So, I tried a bunch of things (including building the libssp libs as llvm-mingw does it). But in all cases it turns out that a) we need the .dll version as all the binaries seem to get linked against it.

If you keep both libssp.dll.a and libssp.a from the build, you'll probably get it linked dynamically. But just get rid of libssp.dll.a and it should end up linking it statically instead. (Or just don't build it - this commit shows what I changed when I made it available as a DLL: https://github.com/mstorsjo/llvm-mingw/commit/d3ec443f9248a83cfbdd4bef311a7191d6acc311#diff-61ff93e7b0a019a6b2791677ea7c6770)

And b) the resulting bundles crash both on 64bit and 32bit Windows systems.

Now that's surprising though. (VLC builds with stack protector enabled via -fstack-protector-strong, with these libs, and runs fine in my tests.)

Comment 3

[Georg Koppen]

(In reply to Martin Storsjö from comment #2)

So, I tried a bunch of things (including building the libssp libs as llvm-mingw does it). But in all cases it turns out that a) we need the .dll version as all the binaries seem to get linked against it.

If you keep both libssp.dll.a and libssp.a from the build, you'll probably get it linked dynamically. But just get rid of libssp.dll.a and it should end up linking it statically instead. (Or just don't build it - this commit shows what I changed when I made it available as a DLL: https://github.com/mstorsjo/llvm-mingw/commit/d3ec443f9248a83cfbdd4bef311a7191d6acc311#diff-61ff93e7b0a019a6b2791677ea7c6770)

And b) the resulting bundles crash both on 64bit and 32bit Windows systems.

Now that's surprising though. (VLC builds with stack protector enabled via -fstack-protector-strong, with these libs, and runs fine in my tests.)

Both a) and b) go away by just copying libssp.a and libssp_nonshared.a, leaving the .dll.a out, thanks! So, we are good for now to test this in our next Tor Browser alpha release I think. (I just copied the libs over from mingw-w64/gcc which we need to build anyway for other parts for Tor Browser).

Comment 4

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Tor got this working in https://gitweb.torproject.org/builders/tor-browser-build.git/commit/?id=562fe8f1df0de8912c9eaf7ff8c3b4d989d4aa01

Comment 5

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1511073 - Enable stack protector for MinGW build r?#Build

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8483ffceb35b
Enable stack protector for MinGW build r=dmajor

Comment 7

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/8483ffceb35b

Comment 8

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1511073 - Enable Stack protector on mingw-clang builds (esr68)

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
Build improvements to the mingw-clang build to reach parity with Tor Browser's current build

User impact if declined: The mingw-clang build on esr68 won't match Tor's. (They use stack protector)

Fix Landed on Version: Nightly 12/10

Risk to taking this patch (and alternatives if risky): The mingw-clang build might break.

Note: Land Bug 1601701 in esr68 before this one.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9115224 [details] [diff] [review]
Bug 1511073 - Enable Stack protector on mingw-clang builds (esr68)

mingw build change for Tor browser parity. NPOTB for mainline Firefox ESR. Approved for 68.4esr.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/697cfb87b3b6