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

Switch from .init_array constructors to /proc/self/auxv. #385

Merged
merged 9 commits into from Aug 8, 2022

Conversation

sunfishcode
Copy link
Member

In the linux_raw backend, switch from using a .init_array constructor
for obtaining the aux values to reading them from /proc/self/auxv. This avoids
problems in situation where other Rust code can run before the constructor,
potentially distrupting the __environ value.

Also, for the linux_raw backend, introduce a new "use-libc-auxv" feature,
which enables use of libc to read the aux values, instead of reading
them from /proc/self/auxv.

The "use-libc-auxv" option is enabled by default, because it's more
efficient and doesn't depend on /proc, so it's likely better for most
users.

Mustang, for its part, continues to be able to use the incoming auxv on
the stack because it controls program startup. Since it doesn't have to
worry about the hazards of /proc or QEMU, it can trust the incoming
values, and do less checking.

Fixes #382.

src/utils.rs Outdated
/// misaligned, or pointing to a region of memory that wraps around the address
/// space.
#[allow(dead_code)]
pub(crate) fn check_raw_pointer<T>(value: *const core::ffi::c_void) -> Option<*const T> {
Copy link

Choose a reason for hiding this comment

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

For my own knowledge, I'm curious if there is any reason not to use NonNull<T> here so we could pack the Option?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only that it hadn't occurred to me :-). But looking into it, NonNull is *mut rather than *const, which is a little inconvenient in src/backend/linux_raw/vdso.rs where it's used a bunch. I'll need to think about this more.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've now added a patch to do this. It is a little more verbose, but I think it's ok.

@cgwalters
Copy link
Contributor

Shouldn't this PR call out a lot more loudly that the default has now switched to libc, and anyone who wants the linux-raw backend needs to opt-in via default-features = false; features = ["linux-raw"]?

It seems worth splitting out into a separate preparatory patch even I'd say.

@cgwalters
Copy link
Contributor

Alternatively, perhaps the implication here due to the indirection implied by a new use-libc-auxv feature is that the default might switch back later. But still, assuming that this is planned to be part of a semver-compatible update it seems at least highlighting somewhat loudly that the backend has switched and isn't likely to switch back soon (right?)

@sunfishcode
Copy link
Member Author

Shouldn't this PR call out a lot more loudly that the default has now switched to libc, and anyone who wants the linux-raw backend needs to opt-in via default-features = false; features = ["linux-raw"]?

The default is still linux_raw; it's just using libc for the aux values, which isn't user-visible. All std-using Rust programs are already linking in libc, on non-Mustang Linux platforms, so adding a dependency on libc by default shouldn't be a breaking change.

It's possible this is a breaking change for no_std users. Rustix has very few no_std users other than Mustang (which isn't affected by this change) and the rustix port of std (which I myself am the only user of and can deal with it). And, rustix's no_std support depends on nightly Rust and gets randomly broken by nightly Rust changes occasionally already. So I'm thinking this doesn't motivate a semver bump.

In the linux_raw backend, switch from using a `.init_array` constructor
for obtaining the aux values to reading them from /proc/self/auxv. This avoids
problems in situation where other Rust code can run before the constructor,
potentially distrupting the `__environ` value.

Also, for the linux_raw backend, introduce a new "use-libc-auxv" feature,
which enables use of libc to read the aux values, instead of reading
them from /proc/self/auxv.

The "use-libc-auxv" option is enabled by default, because it's more
efficient and doesn't depend on /proc, so it's likely better for most
users.

Mustang, for its part, continues to be able to use the incoming auxv on
the stack because it controls program startup. Since it doesn't have to
worry about the hazards of /proc or QEMU, it can trust the incoming
values, and do less checking.

Fixes #382.
This allows it to pack the return value into a single pointer-sized
value.
@sunfishcode sunfishcode merged commit a2c6c7a into main Aug 8, 2022
@sunfishcode sunfishcode deleted the sunfishcode/proc-self-auxv branch August 8, 2022 18:06
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.

Make it possible to selectively use libc for param mod
3 participants