Add dynamicbase flag (build with ASLR when available)

RESOLVED FIXED

Status

()

Core
Build Config
P1
normal
RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: Window Snyder, Assigned: ted)

Tracking

(Blocks: 1 bug)

Trunk
x86
Windows Vista
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:want?])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
Investigate linking with /dynamicbase (ASLR in Vista.)
As noted in the other bug, /dynamicbase isn't supported by MSVC8, only MSVC8SP1, and isn't documented on MSDN (only noted in a couple of security blogs)... this makes me nervous, and it's also pretty complex to write a configure test for.

Comment 2

9 years ago
http://blogs.msdn.com/michael_howard/archive/2006/10/04/Alleged-Bugs-in-Windows-Vista_1920_s-ASLR-Implementation.aspx

it is documented:
http://msdn2.microsoft.com/en-us/library/bb384887.aspx
Component: Build Config → Build Config
Product: Firefox → Core
QA Contact: build.config → build-config
Version: unspecified → Trunk
Does the ref platform use SP1 now?  Is that risky or can we get this?  I'm more than ok with bumping requirements to MSVC8 SP1 if it gets us better mitigation against attacks.  This seems like a solid win for minimal effort.
OS: Mac OS X → Windows Vista
(Assignee)

Comment 4

9 years ago
The refplatform uses SP1. Should be easy to turn this on if you want it.
I don't want to turn this on without a configure check so that we don't break builders with plain VC8 or VC71

Updated

9 years ago
Summary: add dynamicbase flag → Add dynamicbase flag (build with ASLR when available)
Whiteboard: [sg:want?]

Updated

9 years ago
Blocks: 368854
(Assignee)

Comment 6

9 years ago
Created attachment 306914 [details] [diff] [review]
use dynamicbase on vc8sp1 or newer

This WFM, although I didn't do a full build with it, I just re-built spidermonkey and verified it linked properly.
Assignee: nobody → ted.mielczarek
Status: NEW → ASSIGNED
Attachment #306914 - Flags: review?(benjamin)
(Assignee)

Comment 7

9 years ago
Comment on attachment 306914 [details] [diff] [review]
use dynamicbase on vc8sp1 or newer

Oops, VC8 only differs from SP1 in the fourth version component. New patch in a bit.
Attachment #306914 - Attachment is obsolete: true
Attachment #306914 - Flags: review?(benjamin)
(Assignee)

Comment 8

9 years ago
Created attachment 306926 [details] [diff] [review]
better

Right, this should not fail on plain VC8.
Attachment #306926 - Flags: review?(benjamin)

Updated

9 years ago
Flags: blocking1.9+
Priority: -- → P1
(Assignee)

Comment 9

9 years ago
Here's a build with this patch if anyone wants to do some testing on Vista:
https://build.mozilla.org/tryserver-builds/2008-03-04_02:44-tmielczarek@mozilla.com-dynamicbase/tmielczarek@mozilla.com-dynamicbase-firefox-try-win32.zip
https://build.mozilla.org/tryserver-builds/2008-03-04_02:44-tmielczarek@mozilla.com-dynamicbase/tmielczarek@mozilla.com-dynamicbase-firefox-try-win32.installer.exe
Attachment #306926 - Flags: review?(benjamin) → review+
(Assignee)

Comment 10

9 years ago
Checked in. Someone should test this to make sure it doesn't cause any bustage on Vista. Guess we'll find out in tomorrow's nightlies!
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Ted: 

1) With this enabled, did the nightlies pass all tests ok?

2) Is there anything beyond your landed "configure.in" changes that we need in release automation to ship with this? 
(Assignee)

Comment 12

9 years ago
John:
1) I haven't seen any evidence otherwise (Talos boxes are fine, and I haven't seen any bugs filed yet)
2) Nope, this gets enabled by default if your compiler supports it

Updated

8 years ago
Blocks: 504250
You need to log in before you can comment on or make changes to this bug.