Skip to content

Commit b9e2ac5

Browse files
committed
sync::mpsc: prevent double free on Drop
This PR is fixing a regression introduced by rust-lang#121646 that can lead to a double free when dropping the channel. The details of the bug can be found in the corresponding crossbeam PR crossbeam-rs/crossbeam#1187 Signed-off-by: Petros Angelatos <[email protected]>
1 parent 9eb6a54 commit b9e2ac5

File tree

2 files changed

+8
-2
lines changed

2 files changed

+8
-2
lines changed

library/std/src/sync/mpmc/list.rs

+7-1
Original file line numberDiff line numberDiff line change
@@ -569,9 +569,15 @@ impl<T> Channel<T> {
569569
// In that case, just wait until it gets initialized.
570570
while block.is_null() {
571571
backoff.spin_heavy();
572-
block = self.head.block.load(Ordering::Acquire);
572+
block = self.head.block.swap(ptr::null_mut(), Ordering::AcqRel);
573573
}
574574
}
575+
// After this point `head.block` is not modified again and it will be deallocated if it's
576+
// non-null. The `Drop` code of the channel, which runs after this function, also attempts
577+
// to deallocate `head.block` if it's non-null. Therefore this function must maintain the
578+
// invariant that if a deallocation of head.block is attemped then it must also be set to
579+
// NULL. Failing to do so will lead to the Drop code attempting a double free. For this
580+
// reason both reads above do an atomic swap instead of a simple atomic load.
575581

576582
unsafe {
577583
// Drop all messages between head and tail and deallocate the heap-allocated blocks.

src/tools/miri/tests/pass/issues/issue-139553.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ fn main() {
3838
// call returns the channel state has tail.index = head.index, tail.block = NULL, and
3939
// head.block != NULL.
4040
t1.join().unwrap();
41-
// 6. The last sender (s1) is dropped here which also attempts to cleanup any data in the
41+
// 6. The last sender (s2) is dropped here which also attempts to cleanup any data in the
4242
// channel. It observes `tail.index = head.index` and so it doesn't attempt to cleanup any
4343
// messages but it also observes that `head.block != NULL` and attempts to deallocate it.
4444
// This is however already deallocated by `discard_all_messages`, leading to a double free.

0 commit comments

Comments
 (0)