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

libbpf-cargo: Wrap generated "unsafe" type in MaybeUninit #672

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

d-e-s-o
Copy link
Collaborator

@d-e-s-o d-e-s-o commented Feb 9, 2024

Certain Rust types that we use in our generated skeleton are not valid for all bit patterns. Bools, for example, are defined to be one byte in size, but their representation allows only for 0 and 1 values to be used in said byte -- everything else is undefined behavior. Enums have similar problems, in that not the entire space of the backing type is necessarily used by variants.
The result is an unsoundness issue in our generated skeletons, because C code could accidentally set an enum to an invalid value and Rust code would exhibit undefined behavior.
This change fixes this problem by wrapping the corresponding attributes in MaybeUninit. In so doing users have to be explicit when accessing them and make sure that the current representation is indeed valid.

Closes: #589

Certain Rust types that we use in our generated skeleton are not valid
for all bit patterns. Bools, for example, are defined to be one byte in
size, but their representation allows only for 0 and 1 values to be used
in said byte -- everything else is undefined behavior. Enums have
similar problems, in that not the entire space of the backing type is
necessarily used by variants.
The result is an unsoundness issue in our generated skeletons, because C
code could accidentally set an enum to an invalid value and Rust code
would exhibit undefined behavior.
This change fixes this problem by wrapping the corresponding attributes
in MaybeUninit. In so doing users have to be explicit when accessing
them and make sure that the current representation is indeed valid.

Closes: libbpf#589
Signed-off-by: Daniel Müller <[email protected]>
@danielocfb danielocfb requested a review from anakryiko February 9, 2024 01:13
@danielocfb danielocfb marked this pull request as draft February 9, 2024 01:27
Copy link
Member

@anakryiko anakryiko left a comment

Choose a reason for hiding this comment

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

still draft state, so you plan to work on it some more?

can we get a small example/test with enum/bool global variables so it would be easy to see how it is meant to be used in Rust code?

@d-e-s-o
Copy link
Collaborator Author

d-e-s-o commented Feb 9, 2024

still draft state, so you plan to work on it some more?

Yeah, will need to fix the tests.

can we get a small example/test with enum/bool global variables so it would be easy to see how it is meant to be used in Rust code?

Will do.

@d-e-s-o d-e-s-o force-pushed the topic/unsound-types branch from 45d8b1c to ce5ac4a Compare February 12, 2024 18:25
@d-e-s-o d-e-s-o marked this pull request as ready for review February 12, 2024 22:43
@d-e-s-o
Copy link
Collaborator Author

d-e-s-o commented Feb 12, 2024

This pull request should be ready for review now, @anakryiko.

@danielocfb danielocfb merged commit 87929ff into libbpf:master Feb 13, 2024
12 checks passed
@d-e-s-o d-e-s-o deleted the topic/unsound-types branch June 24, 2024 21:47
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.

Potential unsoundness in libbpf-cargo generated code
3 participants