diff options
| author | Petros Angelatos <petrosagg@gmail.com> | 2025-04-10 11:09:31 +0300 |
|---|---|---|
| committer | Petros Angelatos <petrosagg@gmail.com> | 2025-04-11 15:23:12 +0300 |
| commit | 9eb6a5446a4e35f48ad22a5b70a74a8badb9fa0d (patch) | |
| tree | 3bcee6abce74e4e8578f81b3c38387a587d6ea04 | |
| parent | 48f89e7659678f91a68c0c2d868180a0036ab32d (diff) | |
| download | rust-9eb6a5446a4e35f48ad22a5b70a74a8badb9fa0d.tar.gz rust-9eb6a5446a4e35f48ad22a5b70a74a8badb9fa0d.zip | |
sync::mpsc: add miri reproducer of double free
Signed-off-by: Petros Angelatos <petrosagg@gmail.com>
| -rw-r--r-- | library/std/src/sync/mpmc/list.rs | 5 | ||||
| -rw-r--r-- | src/tools/miri/tests/pass/issues/issue-139553.rs | 45 |
2 files changed, 50 insertions, 0 deletions
diff --git a/library/std/src/sync/mpmc/list.rs b/library/std/src/sync/mpmc/list.rs index d88914f5291..2dd8f41226d 100644 --- a/library/std/src/sync/mpmc/list.rs +++ b/library/std/src/sync/mpmc/list.rs @@ -213,6 +213,11 @@ impl<T> Channel<T> { .compare_exchange(block, new, Ordering::Release, Ordering::Relaxed) .is_ok() { + // This yield point leaves the channel in a half-initialized state where the + // tail.block pointer is set but the head.block is not. This is used to + // facilitate the test in src/tools/miri/tests/pass/issues/issue-139553.rs + #[cfg(miri)] + crate::thread::yield_now(); self.head.block.store(new, Ordering::Release); block = new; } else { diff --git a/src/tools/miri/tests/pass/issues/issue-139553.rs b/src/tools/miri/tests/pass/issues/issue-139553.rs new file mode 100644 index 00000000000..97cb9e1a805 --- /dev/null +++ b/src/tools/miri/tests/pass/issues/issue-139553.rs @@ -0,0 +1,45 @@ +//@compile-flags: -Zmiri-preemption-rate=0 -Zmiri-compare-exchange-weak-failure-rate=0 +use std::sync::mpsc::channel; +use std::thread; + +/// This test aims to trigger a race condition that causes a double free in the unbounded channel +/// implementation. The test relies on a particular thread scheduling to happen as annotated by the +/// comments below. +fn main() { + let (s1, r) = channel::<u64>(); + let s2 = s1.clone(); + + let t1 = thread::spawn(move || { + // 1. The first action executed is an attempt to send the first value in the channel. This + // will begin to initialize the channel but will stop at a critical momement as + // indicated by the `yield_now()` call in the `start_send` method of the implementation. + let _ = s1.send(42); + // 4. The sender is re-scheduled and it finishes the initialization of the channel by + // setting head.block to the same value as tail.block. It then proceeds to publish its + // value but observes that the channel has already disconnected (due to the concurrent + // call of `discard_all_messages`) and aborts the send. + }); + std::thread::yield_now(); + + // 2. A second sender attempts to send a value while the channel is in a half-initialized + // state. Here, half-initialized means that the `tail.block` pointer points to a valid block + // but `head.block` is still null. This condition is ensured by the yield of step 1. When + // this call returns the channel state has tail.index != head.index, tail.block != NULL, and + // head.block = NULL. + s2.send(42).unwrap(); + // 3. This thread continues with dropping the one and only receiver. When all receivers are + // gone `discard_all_messages` will attempt to drop all currently sent values and + // de-allocate all the blocks. If `tail.block != NULL` but `head.block = NULL` the + // implementation waits for the initializing sender to finish by spinning/yielding. + drop(r); + // 5. This thread is rescheduled and `discard_all_messages` observes the head.block pointer set + // by step 4 and proceeds with deallocation. In the problematic version of the code + // `head.block` is simply read via an `Acquire` load and not swapped with NULL. After this + // call returns the channel state has tail.index = head.index, tail.block = NULL, and + // head.block != NULL. + t1.join().unwrap(); + // 6. The last sender (s1) is dropped here which also attempts to cleanup any data in the + // channel. It observes `tail.index = head.index` and so it doesn't attempt to cleanup any + // messages but it also observes that `head.block != NULL` and attempts to deallocate it. + // This is however already deallocated by `discard_all_messages`, leading to a double free. +} |
