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

Unprivileged objNameAllowsDot, return *btf.Var from Variable{Spec}.Type() #1612

Merged
merged 3 commits into from
Nov 14, 2024

Conversation

ti-mo
Copy link
Collaborator

@ti-mo ti-mo commented Nov 14, 2024

Fixes for a few papercuts encountered while implementing the API into the Cilium agent.

Changed my mind about returning btf.Type from .Type(), this should just be *btf.Var. No need in obscuring the Var, it's the easiest way to get to decl tags.

When loading ELFs using unprivileged tools or tests, EPERM causes dots and
underscores to be stripped from MapSpec.Name, causing strange results for
maps like .rodata.foo.

Also, only consider ErrNotSupported as conclusive and bubble up any other
errors to the caller. Most callers just check err != nil, but in case
someone indirectly relies on the sentinel, this should result in more
accurate output.

Signed-off-by: Timo Beckers <[email protected]>
@ti-mo ti-mo requested a review from a team as a code owner November 14, 2024 14:17
@github-actions github-actions bot added the breaking-change Changes exported API label Nov 14, 2024
In the initial design, we decided to return the inner type from .Type(), but
I forgot the outer btf.Var was needed to access decl tags set on the variable.

Returning a *Var here is the most convenient way of accessing the tags, otherwise
the caller would need to find the underlying map and parse the datasec manually,
which goes against what this API aims to solve.

Instead of the initial idea of returning a btf.Var as btf.Type, this now returns
btf.Var directly.

Signed-off-by: Timo Beckers <[email protected]>
Cilium's variable inliner needs this information to look up variable contents
based on the offset specified in the instruction stream, and wants to validate
that it's pulling data from the right data section.

It makes for a slightly leaky abstraction, but at least something meaningful
can be accomplished by combining the two, e.g. by accessing the given offset
in the map.

Signed-off-by: Timo Beckers <[email protected]>
@ti-mo ti-mo removed the breaking-change Changes exported API label Nov 14, 2024
@ti-mo
Copy link
Collaborator Author

ti-mo commented Nov 14, 2024

Removed breaking-change label as this API is yet to appear in a release.

@ti-mo ti-mo merged commit ec48f59 into cilium:main Nov 14, 2024
17 checks passed
@ti-mo ti-mo deleted the tb/variable-followups branch November 14, 2024 14:30
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.

1 participant