Skip to content

Commit 5fb6511

Browse files
committed
zeroize: Remove CPU fences
Per @RalfJung: #214 (comment) > AFAIK atomic fences only really prevent reordering of atomic accesses. > Non-atomic accesses can still be reordered. The interaction of this > with volatile accesses is entirely unclear -- technically, they are > non-atomic, but in practice it seems unlikely that the compiler would > perform such reorderings. > [...] > A compiler fence seems like a reasonable precaution and also helps > make sure the side-effects happens near where the programmer might > expect it, but a CPU fence to me sounds more like cargo cult.
1 parent e943be1 commit 5fb6511

File tree

1 file changed

+10
-18
lines changed

1 file changed

+10
-18
lines changed

zeroize/src/lib.rs

+10-18
Original file line numberDiff line numberDiff line change
@@ -124,19 +124,15 @@
124124
//! [these remarks have been removed] and the specific usage pattern in this
125125
//! crate is considered to be well-defined.
126126
//!
127-
//! To help mitigate concerns about reordering of operations executed by the
128-
//! CPU potentially exposing values after they have been zeroed, this crate
129-
//! leverages the [core::sync::atomic] memory fence functions including
130-
//! [compiler_fence] and [fence] (which uses the CPU's native fence
131-
//! instructions). These fences are leveraged with the strictest ordering
132-
//! guarantees, [Ordering::SeqCst], which ensures no accesses are reordered.
127+
//! Additionally this crate leverages [compiler_fence] from
128+
//! [core::sync::atomic] with the strictest ordering ([Ordering::SeqCst])
129+
//! as a precaution to help ensure reads are not reordered before memory has
130+
//! been zeroed.
133131
//!
134132
//! All of that said, there is still potential for microarchitectural attacks
135-
//! (ala Spectre/Meltdown) to leak "zeroized" secrets through covert channels
136-
//! (e.g. the memory fences mentioned above have previously been used as a
137-
//! covert channel in the Foreshadow attack). This crate makes no guarantees
138-
//! that zeroized values cannot be leaked through such channels, as they
139-
//! represent flaws in the underlying hardware.
133+
//! (ala Spectre/Meltdown) to leak "zeroized" secrets through covert channels.
134+
//! This crate makes no guarantees that zeroized values cannot be leaked
135+
//! through such channels, as they represent flaws in the underlying hardware.
140136
//!
141137
//! ## Stack/Heap Zeroing Notes
142138
//!
@@ -196,7 +192,6 @@
196192
//! [core::sync::atomic]: https://doc.rust-lang.org/stable/core/sync/atomic/index.html
197193
//! [Ordering::SeqCst]: https://doc.rust-lang.org/std/sync/atomic/enum.Ordering.html#variant.SeqCst
198194
//! [compiler_fence]: https://doc.rust-lang.org/stable/core/sync/atomic/fn.compiler_fence.html
199-
//! [fence]: https://doc.rust-lang.org/stable/core/sync/atomic/fn.fence.html
200195
//! [memory-model]: https://github.com/nikomatsakis/rust-memory-model
201196
//! [unordered]: https://llvm.org/docs/Atomics.html#unordered
202197
//! [llvm-atomic]: https://github.com/rust-lang/rust/issues/58599
@@ -305,11 +300,9 @@ where
305300

306301
/// Implement `Zeroize` on slices of types that can be zeroized with `Default`.
307302
///
308-
/// This impl can eventually be optimized using an atomic memset intrinsic,
309-
/// such as `llvm.memset.element.unordered.atomic`. For that reason the blanket
310-
/// impl on slices is bounded by `DefaultIsZeroes`. See:
311-
///
312-
/// <https://github.com/rust-lang/rust/issues/58599>
303+
/// This impl can eventually be optimized using an memset intrinsic,
304+
/// such as `core::intrinsics::volatile_set_memory`. For that reason the blanket
305+
/// impl on slices is bounded by `DefaultIsZeroes`.
313306
///
314307
/// To zeroize a mut slice of `Z: Zeroize` which does not impl
315308
/// `DefaultIsZeroes`, call `iter_mut().zeroize()`.
@@ -404,7 +397,6 @@ where
404397
/// see zeroes after this point.
405398
#[inline]
406399
fn atomic_fence() {
407-
atomic::fence(atomic::Ordering::SeqCst);
408400
atomic::compiler_fence(atomic::Ordering::SeqCst);
409401
}
410402

0 commit comments

Comments
 (0)