Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: enable PIE by default #8792

Merged
merged 3 commits into from
Oct 31, 2019
Merged

build: enable PIE by default #8792

merged 3 commits into from
Oct 31, 2019

Conversation

lizan
Copy link
Member

@lizan lizan commented Oct 29, 2019

Signed-off-by: Lizan Zhou lizan@tetrate.io

Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
@lizan lizan requested a review from htuch October 29, 2019 00:19
@lizan
Copy link
Member Author

lizan commented Oct 29, 2019

@PiotrSikora @agl @davidben is it possible to add -fPIC to BoringSSL FIPS build?

@PiotrSikora
Copy link
Contributor

PiotrSikora commented Oct 29, 2019

As far as I know, we can't, since the Security Policy has to be followed to the dot when building module... but @agl is the authority here.

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lizan, this is a very helpful improvement.

.bazelrc Show resolved Hide resolved
@mattklein123
Copy link
Member

Can we add a release note also about this? I think it's useful to let people know that we expect the binary to be runnable under ASLR.

Signed-off-by: Lizan Zhou <lizan@tetrate.io>
@lizan lizan marked this pull request as ready for review October 29, 2019 21:27
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some nits. Thanks.
/wait

test/exe/pie_test.sh Outdated Show resolved Hide resolved
test/exe/pie_test.sh Outdated Show resolved Hide resolved
test/exe/pie_test.sh Show resolved Hide resolved
test/exe/pie_test.sh Show resolved Hide resolved
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
@lizan
Copy link
Member Author

lizan commented Oct 30, 2019

/retest

@repokitteh-read-only
Copy link

🔨 rebuilding ci/circleci: coverage (failed build)

🐱

Caused by: a #8792 (comment) was created by @lizan.

see: more, trace.

Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@htuch htuch merged commit c44ac4b into envoyproxy:master Oct 31, 2019
@nareddyt
Copy link
Contributor

nareddyt commented Nov 7, 2019

HI @lizan, could you explain the -fPIC copt you added to .bazelrc? I get what the copt does, but I don't understand why it's needed to build now.

@lizan
Copy link
Member Author

lizan commented Nov 7, 2019

@nareddyt to link a binary with -pie all objects (.o files) has to be compiled with -fPIC. --copt is propagating all the way down to dependencies including foreign_cc cmake rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants