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.

Sign up for GitHub

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

Make stack non-executable #105

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Make stack non-executable #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4809ac0
Make stack non-executable
cohosh May 13, 2019
90a6f2a
Update .travis.yml to allow LD flags
cohosh May 14, 2019
2827831
Add target-specfic flags for ld
cohosh May 21, 2019
395e4e5
Merge branch 'ldflags' of github.com:cohosh/go-webrtc into ldflags
cohosh May 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ matrix:
dist: xenial
sudo: required
go: 1.6
env: CC="gcc-5" CXX="g++-5" OS="linux"
env: CC="gcc-5" CXX="g++-5" OS="linux" CGO_LDFLAGS_ALLOW="-z|noexecstack"
- os: osx
env: CGO_LDFLAGS_ALLOW="-z|noexecstack"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this doing anything now?


install:
- go get -u github.com/smartystreets/goconvey
Expand Down
3 changes: 2 additions & 1 deletion datachannel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ package webrtc

/*
#cgo CXXFLAGS: -std=c++0x
#cgo LDFLAGS: -L${SRCDIR}/lib
#cgo linux LDFLAGS: -L${SRCDIR}/lib -z noexecstack
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about "android"?

#cgo darwin LDFLAGS: -L${SRCDIR}/lib
#cgo android pkg-config: webrtc-android-armeabi-v7a.pc
#cgo linux,arm pkg-config: webrtc-linux-arm.pc
#cgo linux,386 pkg-config: webrtc-linux-386.pc
Expand Down
3 changes: 2 additions & 1 deletion peerconnection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ package webrtc

/*
#cgo CXXFLAGS: -std=c++0x
#cgo LDFLAGS: -L${SRCDIR}/lib
#cgo linux LDFLAGS: -L${SRCDIR}/lib -z noexecstack
#cgo darwin LDFLAGS: -L${SRCDIR}/lib
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not necessarily opposed to these changes but do these flags need to be set at the source level?

Would something like this,

CGO_LDFLAGS_ALLOW="-z|noexecstack" go build -ldflags '-z noexecstack'

work here?

https://gitweb.torproject.org/builders/tor-browser-build.git/tree/projects/snowflake/build#n46

Copy link
Author

Choose a reason for hiding this comment

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

Actually, even simpler, you can set
export CGO_LDFLAGS="-z noexecstack"
I think we should do this for the Tor Browser build.

It worries me that the stack is executable by default, but this is probably something that should be fixed with the go linker rather than here. The advantage to doing everything in environment variables or with -ldflags as you suggested is you don't need to remember to set things in more than one place.

I'm definitely good if the decision is to not merge this pull request.

#cgo android pkg-config: webrtc-android-armeabi-v7a.pc
#cgo linux,arm pkg-config: webrtc-linux-arm.pc
#cgo linux,386 pkg-config: webrtc-linux-386.pc
Expand Down