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

Jump to bottom

Check unsafe.Pointer use with go 1.14 -d=checkptr compiler option #10133

Closed
tklauser opened this issue Feb 11, 2020 · 6 comments
Closed

Check unsafe.Pointer use with go 1.14 -d=checkptr compiler option #10133

tklauser opened this issue Feb 11, 2020 · 6 comments
Labels
kind/cleanup This includes no functional changes. stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.

Comments

@tklauser
Copy link
Member

The upcoming Go 1.14 release will include -d=checkptr as a compile-time option for adding instrumentation to check that Go code is following unsafe.Pointer safety rules dynamically. This might help in finding problematic uses of unsafe.Pointer in Cilium's code base, e.g. causing memory corruption.

This option should probably not be enabled by default, but could e.g. be run as part of CI.

Also see https://tip.golang.org/doc/go1.14#compiler for details.

/cc @brb
@tklauser tklauser added the kind/cleanup This includes no functional changes. label Feb 11, 2020
tklauser added a commit that referenced this issue Feb 11, 2020
@tklauser
vendor: re-vendor golang.org/x/sys and github.com/vishvananda/netlink
8dd3a8c 
This pulls in several unsafe.Pointer usage fixes found in these packages
using the Go 1.14 `-d=checkptr` compiler flag.

Updates #10133

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@tklauser tklauser mentioned this issue Feb 11, 2020
Merged
@tklauser
Copy link
Member Author

I also started checking our dependencies and sent some PRs already:

Probably more to come.

@brb
Copy link
Member

brb commented Feb 13, 2020

@tklauser Cool, have you already tried compiling with checkptr cilium-agent?

@tklauser
Copy link
Member Author

@brb not yet, so far I only ran it with the tests. Will try cilium-agent next.

aanm pushed a commit that referenced this issue Feb 19, 2020
@tklauser @aanm
vendor: re-vendor golang.org/x/sys and github.com/vishvananda/netlink
84d5af3 
This pulls in several unsafe.Pointer usage fixes found in these packages
using the Go 1.14 `-d=checkptr` compiler flag.

Updates #10133

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@tklauser tklauser mentioned this issue Mar 31, 2020
Closed
tklauser added a commit that referenced this issue Mar 31, 2020
@tklauser
Enable unsafe.Pointer use with checkptr flag in CI test binaries
6742ccb 
DO NOT MERGE, testing only.

Check `unsafe.Pointer` using the Go 1.14 `-d=checkptr` compiler option
in CI test builds.

For #10133

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@stale
Copy link

stale bot commented Apr 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Apr 13, 2020
@tklauser tklauser removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Apr 13, 2020
tklauser added a commit that referenced this issue Apr 24, 2020
@tklauser
Switch from pkg.in/inotify.v1 to github.com/fsnotify/fsnotify
ef752ed 
The fsnotify package suggests to use the github.com/fsnotify/fsnotify
package patch, see fsnotify/fsnotify#108.

While at it also update to the current master of
github.com/fsnotify/fsnotify in order to fix an unsafe pointer
conversion for #10133

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@tklauser tklauser mentioned this issue Apr 24, 2020
Merged
aanm pushed a commit that referenced this issue Apr 27, 2020
@tklauser @aanm
Switch from pkg.in/inotify.v1 to github.com/fsnotify/fsnotify
a33d5cf 
The fsnotify package suggests to use the github.com/fsnotify/fsnotify
package patch, see fsnotify/fsnotify#108.

While at it also update to the current master of
github.com/fsnotify/fsnotify in order to fix an unsafe pointer
conversion for #10133

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@stale
Copy link

stale bot commented Jun 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jun 12, 2020
@stale
Copy link

stale bot commented Jun 26, 2020

This issue has not seen any activity since it was marked stale. Closing.

@stale stale bot closed this as completed Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/cleanup This includes no functional changes. stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@brb @tklauser