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

map: replace hacky unsafe.Pointer conversion with PROT_NONE page #915

Merged
merged 1 commit into from
Jan 26, 2023

Conversation

lmb
Copy link
Collaborator

@lmb lmb commented Jan 24, 2023

One of our legacy code paths currently does a dodgy pointer conversion:

unsafe.Pointer(^uintptr(0))

This is (correctly) flagged by linters as being a bit bogus, since unsafe.Pointer conversions do have an effect on the GC (I believe). It also assumes that the highest address is never used.

Map an inaccessible page and use that instead of the bogus pointer. This has the same effect but is much safer.

Signed-off-by: Lorenz Bauer oss@lmb.io

@lmb
Copy link
Collaborator Author

lmb commented Jan 24, 2023

cc @nplanel

@lmb lmb requested a review from nplanel January 24, 2023 10:50
One of our legacy code paths currently does a dodgy pointer conversion:

    unsafe.Pointer(^uintptr(0))

This is (correctly) flagged by linters as being a bit bogus, since
unsafe.Pointer conversions do have an effect on the GC (I believe).
It also assumes that the highest address is never used.

Map an inaccessible page and use that instead of the bogus pointer.
This has the same effect but is much safer.

Signed-off-by: Lorenz Bauer <oss@lmb.io>
Copy link
Collaborator

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

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

Nice! This has been bothering me since forever.

@lmb lmb merged commit 4ac4bbd into cilium:master Jan 26, 2023
@lmb lmb deleted the map-guess-mmap branch January 26, 2023 11:56
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.

2 participants