Possible UB in Zeroize implementation for Option<T>

[jessa0]

It seems the Zeroize implementation for Option<T> where T: Zeroize has language-level UB here:

crates/zeroize/src/lib.rs

Line 317 in f6deffc

volatile_set(self as *mut _ as *mut u8, 0, core::mem::size_of::<Self>());

I believe, as a repr(Rust) enum, the memory layout and set of valid bit-patterns for Option is not defined, and that setting an enum's storage to an invalid bit-pattern while a reference to it exists, even if the value is never read, is instant language-level UB. The documentation for Option does mention guarantees for several special cases, but the None case still isn't defined for many of those cases, and the Zeroize implementation is more generic than that. Here's an example of a miri error in such a situation, that scottmcm came up with on URLO here.

[tony-iqlusion]

Thanks for reporting this. I was a bit hesitant about this in regard to PRs like #649 and #687, especially as I read ongoing discussion about Option padding like https://internals.rust-lang.org/t/allow-option-to-have-its-padding-explicitly-reused/14941

Unfortunately I don't have anything more to say about this topic, but can circulate this question among interested parties, particularly ones who work on unsafe code guidelines to get more feedback.

[jessa0]

FWIW, the language reference only mentions wrt. invalid enum discriminants, "Producing an invalid value, even in private fields and locals [is undefined behaviour]. 'Producing' a value happens any time a value is assigned to or read from a place, passed to a function/primitive operation or returned from a function/primitive operation." This to me, sounds like, despite an outstanding mutable reference, changing the underlying storage to an invalid value without reading it back through the reference is still defined. That same page in the reference of course mentions that the list of UB is not considered exhaustive, though.

[jessa0]

I found this discussion in the Unsafe Code Guidelines WG which seems to almost match this code. It might be worth bringing up whether ptr::write_volatile changes the equation at all.

[tony-iqlusion]

Moved to RustCrypto/utils#653