Skip to content

src: only disable PIE on OS X when -prof is passed - #6468

Closed
bnoordhuis wants to merge 2 commits into
nodejs:masterfrom
bnoordhuis:fix6466
Closed

bnoordhuis wants to merge 2 commits into
nodejs:masterfrom
bnoordhuis:fix6466

Conversation

@bnoordhuis

@bnoordhuis bnoordhuis commented Apr 29, 2016 •

Copy link
Copy Markdown
Member

This is a follow-up to commit a5012a0 ("build: unbreak -prof, disable
PIE on OS X") that disabled PIE (and therefore ASLR) completely on OS X.

We now scan the command line options and re-exec the process with the
_POSIX_SPAWN_DISABLE_ASLR flag set when -prof is specified.

The options parser is smart enough to understand any combination of
-prof and -noprof, including alternative spellings like --no_prof.

Fixes: #6466

CI: https://ci.nodejs.org/job/node-test-pull-request/2429/

This is a follow-up to commit a5012a0 ("build: unbreak -prof, disable
PIE on OS X") that disabled PIE (and therefore ASLR) completely on OS X.

We now scan the command line options and re-exec the process with the
_POSIX_SPAWN_DISABLE_ASLR flag set when `-prof` is specified.

The options parser is smart enough to understand any combination of
`-prof` and `-noprof`, including alternative spellings like `--no_prof`.

Fixes: nodejs#6466
@bnoordhuis bnoordhuis added build Issues and PRs related to Node.js builds or CI infrastructure. macos Issues and PRs related to the macOS platform. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. security Issues and PRs related to security. labels Apr 29, 2016
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Apr 29, 2016
@jasnell

jasnell commented Apr 29, 2016

Copy link
Copy Markdown
Member

Nice! Will give it a review and test here in a bit

@jasnell

jasnell commented Apr 29, 2016

Copy link
Copy Markdown
Member

LGTM. CI is green. Local test run looks good too.

@bnoordhuis ... Question: I'm not too familiar with posix_spawnattr_setflags ... is it possible / worthwhile to check to see if the _POSIX_SPAWN_DISABLE_ASLR was already set to avoid the respawn?

@bnoordhuis

Copy link
Copy Markdown
Member Author

@jasnell I've added a commit that does that.

@bnoordhuis

Copy link
Copy Markdown
Member Author

Closing, obsoleted by https://codereview.chromium.org/1934453003.

@bnoordhuis bnoordhuis closed this May 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Issues and PRs related to Node.js builds or CI infrastructure. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. macos Issues and PRs related to the macOS platform. security Issues and PRs related to security.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants