search

rust-lang / futures-rs

Zero-cost asynchronous programming in Rust
https://rust-lang.github.io/futures-rs/
Apache License 2.0
5.31k stars 609 forks source link

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

Closed Bromind closed 9 months ago

Bromind commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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.