8000
Skip to content

*: switch to -buildmode=pie - #34369

Merged
AkihiroSuda merged 1 commit into
moby:masterfrom
cyphar:build-buildmode-pie
Jan 29, 2018
Merged

*: switch to -buildmode=pie#34369
AkihiroSuda merged 1 commit into
moby:masterfrom
cyphar:build-buildmode-pie

Conversation

@cyphar
@cyphar cyphar commented Aug 2, 2017
Copy link
Copy Markdown
Contributor

Go has supported PIC builds for a while now, and given the security
benefits of using PIC binaries we should really enable them. There also
appears to be some indication that non-PIC builds have been interacting
oddly on ppc64le (the linker cannot load some shared libraries), and
using PIC builds appears to solve this problem.

Hedgehog Ears!

Hedgehog Ears! by Christian Heilmann

Signed-off-by: Aleksa Sarai asarai@suse.de

@cyphar
cyphar requested review from dnephin and tianon as code owners August 2, 2017 13:29
@justincormack
justincormack commented Aug 2, 2017
Copy link
Copy Markdown
Contributor

As we have cgo code, to get relocatable binaries so ASLR can work we may need -extldflags "-fno-PIC" as well?

@cyphar
cyphar commented Aug 2, 2017
Copy link
Copy Markdown
Contributor Author

Are you sure you meant -fno-PIC and not -fPIC?

@AkihiroSuda
AkihiroSuda commented Aug 3, 2017
Copy link
Copy Markdown
Member

CI failing

00:04:29.101 ---> Making bundle: binary-daemon (in bundles/17.06.0-dev/binary-daemon)
00:04:29.321 Building: bundles/17.06.0-dev/binary-daemon/dockerd-17.06.0-dev
00:05:35.280 # github.com/docker/docker/cmd/dockerd
00:05:35.280 /usr/local/go/pkg/tool/linux_amd64/link: running gcc failed: exit status 1
00:05:35.281 /usr/bin/ld: /usr/lib/gcc/x86_64-linux-gnu/4.9/crtbeginT.o: relocation R_X86_64_32 against `__TMC_END__' can not be used when making a shared object; recompile with -fPIC
00:05:35.281 /usr/lib/gcc/x86_64-linux-gnu/4.9/crtbeginT.o: error adding symbols: Bad value
00:05:35.281 collect2: error: ld returned 1 exit status
00:05:35.281

@AkihiroSuda AkihiroSuda added status/1-design-review status/failing-ci Indicates that the PR in its current state fails the test suite and removed status/0-triage labels Aug 3, 2017
@cyphar
cyphar commented Aug 3, 2017
Copy link
Copy Markdown
Contributor Author

I think the CI is failing because of the packages installed on the system, but it might also be a missing -fPIC.

@AkihiroSuda
AkihiroSuda commented Sep 19, 2017
Copy link
Copy Markdown
Member

ping @cyphar

@AkihiroSuda
AkihiroSuda commented Nov 10, 2017
Copy link
Copy Markdown
Member

ping @cyphar @justincormack

@cyphar
cyphar commented Nov 10, 2017
Copy link
Copy Markdown
Contributor Author

/me will rebase and re-review today.

@cyphar
cyphar force-pushed the build-buildmode-pie branch from a757a18 to 0a57b63 Compare November 11, 2017 08:05
@cyphar
cyphar requested a review from AkihiroSuda as a code owner November 11, 2017 08:05
@cyphar
*: switch to -buildmode=pie
1f4e37c 
Go has supported PIC builds for a while now, and given the security
benefits of using PIC binaries we should really enable them. There also
appears to be some indication that non-PIC builds have been interacting
oddly on ppc64le (the linker cannot load some shared libraries), and
using PIC builds appears to solve this problem.

Signed-off-by: Aleksa Sarai <asarai@suse.de>
@cyphar
cyphar force-pushed the build-buildmode-pie branch from 0a57b63 to 1f4e37c Compare November 11, 2017 12:58
@cyphar
cyphar commented Nov 11, 2017
Copy link
Copy Markdown
Contributor Author

All the tests pass, @AkihiroSuda.

@thaJeztah thaJeztah removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Nov 11, 2017
@thaJeztah
thaJeztah commented Nov 11, 2017
Copy link
Copy Markdown
Member

ping @tianon @justincormack PTAL

@cyphar
cyphar commented Dec 7, 2017
Copy link
Copy Markdown
Contributor Author

/ping ?

@AkihiroSuda
AkihiroSuda merged commit cd3c005 into moby:master Jan 29, 2018
@cyphar
cyphar deleted the build-buildmode-pie branch February 1, 2018 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes
@dnephin dnephin Awaiting requested review from dnephin
@tianon tianon Awaiting requested review from tianon
+1 more reviewer
@justincormack justincormack justincormack approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@aluzzardi aluzzardi

Labels

status/1-design-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@cyphar @justincormack @AkihiroSuda @thaJeztah @aluzzardi @GordonTheTurtle
0