Description
Hello, I looked at select_slice implementation and found out that it is unsound.
Here is an example of 100% safe code that causes miri to report UB (pollster is just a block_on implementation, you can reproduce it with tokio, it does not matter):
use std::mem::swap;
use embassy_futures::{select::select_slice, yield_now};
async fn wait_for(id: usize, count: usize) {
let reference = &id;
for _ in 0..count {
yield_now().await;
}
println!("{}", reference);
}
fn main() {
let future1 = wait_for(1, 1);
let future2 = wait_for(2, 0);
let mut futures_1 = [future1, future2];
let ((), idx) = pollster::block_on(select_slice(&mut futures_1[..]));
assert_eq!(idx, 1);
let mut future3 = wait_for(3, 1);
swap(&mut futures_1[0], &mut future3);
pollster::block_on(future3);
}Output:
> cargo miri run
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.03s
Running `/home/ddystopia/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner target/miri/x86_64-unknown-linux-gnu/debug/play`
2
error: Undefined Behavior: trying to retag from <3569> for SharedReadOnly permission at alloc1343[0x20], but that tag does not exist in the borrow stack for this location
--> src/main.rs:24:24
|
24 | pollster::block_on(future3);
| ^^^^^^^
| |
| trying to retag from <3569> for SharedReadOnly permission at alloc1343[0x20], but that tag does not exist in the borrow stack for this location
| this error occurs as part of retag (of a reference/box inside this compound value) at alloc1343[0x20..0x28]
| errors for retagging in fields are fairly new; please reach out to us (e.g. at <https://rust-lang.zulipchat.com/#narrow/stream/269128-miri>) if you find this error troubling
|
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <3569> was created by a SharedReadOnly retag at offsets [0x20..0x28]
--> src/main.rs:6:21
|
6 | let reference = &id;
| ^^^
help: <3569> was later invalidated at offsets [0x0..0x38] by a write access
--> src/main.rs:22:5
|
22 | swap(&mut futures_1[0], &mut future3);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: BACKTRACE (of the first span):
= note: inside `main` at src/main.rs:24:24: 24:31
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
error: aborting due to 1 previous error
or, without miri, program outputs garbage:
> cargo run
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.01s
Running `target/debug/play`
2
107725204834832
> cargo run
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.01s
Running `target/debug/play`
2
104986498681360
> cargo run
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.01s
Running `target/debug/play`
2
> cargo run
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.01s
Running `target/debug/play`
2
95884436826640
The issue is that we a accepting slice by mutable reference, then constructing pin around each future and poll. But then select_slice returns all futures to the user, unpinned again.
select actually is sound because it takes ownership of both futures. Then Select future is pinned before poll, and then it projects its pin onto owned futures. When it's done, second future is not returned to the user - it will be either leaked or dropped.
My vision on how to solve it is accept Pin<&'a mut [Fut]> instead of &'a mut [Fut]. That way user cannot safely do anything to future if it is Unpin.
Activity