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

futures-util does not compile if size of usize is not size of raw pointers #2777

Closed
Bromind opened this issue Oct 6, 2023 · 5 comments
Closed

Comments

@Bromind
Copy link

Bromind commented Oct 6, 2023

The current implementation of BiLocks internally uses core::mem::transmute, which requires that the source and destination types have the same size ("Both types must have the same size", from transmute doc). In particular, transmute is used to convert usize into *mut T (https://github.com/rust-lang/futures-rs/blob/2c1c3e152f4639711658be341e00cca4cad9aa5e/futures-util/src/lock/bilock.rs#L292C13-L292C34).

Also, BiLocks are compiled as part of futures-util even if the bilock feature is not set, as they are used internally. Therefore, futures-util does not compile on architectures where usize do not have the same size as raw pointers (e.g. architectures with capability pointers : https://www.cl.cam.ac.uk/research/security/ctsrd/cheri/ ).

I think the new implementation proposed in PR #2384 does not suffer this issue.

@taiki-e
Copy link
Member

taiki-e commented Oct 6, 2023

As explained in the comments, it is code derived from the standard library, and the same is used in crates like sptr. Currently, there is no way to work well with strict_provenance without such a helper, so it is not possible to change it.

The right place to progress this is the standard library and stabilize strict_provenance there. Once strict_provenance is stable, we can use it to handle this automatically.

There is nothing we can do about this at this time on our end, so we will close this, but feel free to reopen it once the strict_provenance is stable.

@taiki-e taiki-e closed this as not planned Won't fix, can't repro, duplicate, stale Oct 6, 2023
@taiki-e
Copy link
Member

taiki-e commented Oct 6, 2023

BTW, IIRC, there was some discussion in the past regarding the compatibility of CHERI with the standard library that it would be reasonable to use the strict_provenance APIs instead of transmutes/casts.

@Bromind
Copy link
Author

Bromind commented Oct 6, 2023

Thanks for the update.

I agree in general, the standard library would be the right place to go, but in this precise case, merging PR #2384 would solve the issue (I just tried to compile the proposed merge targeting aarch64-unknown-freebsd-purecap, which worked out of the box).

Is there any reason I'm missing not to merge it? (If need be, I'm willing to put some work into it, e.g. if there is a merge conflict).

@taiki-e
Copy link
Member

taiki-e commented Oct 9, 2023

in this precise case, merging PR #2384 would solve the issue

I think this does not help to solve the underlying problem. Similar strict_provenance helpers are also used in several important places in the ecosystem, and it is not realistic to rewrite the algorithm for all code that uses them. To make more code CHERI-compatible, I think it is reasonable to introduce a CHERI-compatible strict_provenance helper, rather than requiring random projects that encounter problems to change their algorithms.

I guess aarch64-unknown-freebsd-purecap has the standard library, but how does it implement the strict_provenance helper?

Is there any reason I'm missing not to merge it? (If need be, I'm willing to put some work into it, e.g. if there is a merge conflict).

There are conflicts, and the latest version of that PR also has a performance issue and a compatibility issue with sanitizers (see my comment in that PR).

@taiki-e
Copy link
Member

taiki-e commented Oct 9, 2023

I guess aarch64-unknown-freebsd-purecap has the standard library, but how does it implement the strict_provenance helper?

I wonder if something like core::ptr::null_mut::<u8>().wrapping_add(addr).cast::<T>() would work.

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

No branches or pull requests

2 participants