RFC: add freeze operation

[honzasp]

View all comments

Introduce an operation similar to the LLVM freeze instruction, which converts uninitialized values into initialized but arbitrary values:

impl<T> MaybeUninit<T> {
    const fn freeze(self) -> MaybeUninit<T>;
}

The biggest disadvantage of adding the freeze operation is that a Rust program can leak a secret that was previously stored in the uninitialized memory without triggering UB.

Important

Since RFCs involve many conversations at once that can be difficult to follow, please use review comment threads on the text changes instead of direct comments on the RFC.

If you don't have a particular section of the RFC to comment on, you can click on the "Comment on this file" button on the top-right corner of the diff, to the right of the "Viewed" checkbox. This will create a separate thread even if others have commented on the file too.

Rendered

[Lokathor]

Hello. Yes, I'd merge this PR for sure.

View changes since the review

[RalfJung]

This sounds very good overall, thanks a lot :)
Cc @rust-lang/opsem

View changes since this review

[RalfJung]

Cc @nikic -- do you think it makes sense to add a flag to LLVM's memcpy that indicates a freeze? Ideally we'd not have to hand-roll our own freezing memcpy in Rust, that seems a bit silly. We want to use the regular highly-optimized memcpy primitive, just in a way that LLVM considers to be freezing all undef/poison.

[RalfJung]

I don't find this very convincing. We don't usually name things after the LLVM IR operation they compile to.

I think it should be called frozen.

[oskgo]

Maybe you shouldn't present use case 1 as the main motivating use case then?

Alternatively you could carve out a specific application where leaking things does not matter. This is going to happen whenever everything that has access to the location you serialize to necessarily has access to all the information accessible in your entire program. Even with this it's still problematic because it increases the attack surface, not just across space but also across time.

I suspect for many their first instinct is going to be to use this to serialize things for sending over the network, and that's not a good idea.

View changes since the review

[honzasp]

I put use case 1 first because it's the use case that motivated me to research this. I agree that people will likely want to use freeze to serialize things sent over the network (like I do), which I think is a good argument against adding freeze into the language.

[hanna-kruppe]

In general, we don't have to make things UB to consider them wrong and try to prevent them. We also have the concept of erroneous behavior (EB), where the program is considered buggy and can be aborted if e.g. run under Miri or a sanitizer, but otherwise has well-defined behavior (or at least not UB).

Could we have EB for at least some subset of Rust programs that leak uninitialized memory? It makes no sense to consider every use of freeze that sees an uninit byte to be EB, if we want that then we shouldn't add freeze to begin with. But tools like Valgrind and MemorySanitizer can diagnose specific uses of uninitialized memory that are likely bugs, e.g., branching on a condition or dereferencing a pointer derived from uninitialized memory.

Is there a way to specify EB that blesses (roughly) the kind of checks those tools perform? That seems like it would provide a decent compromise (it rules out use case 1 but still allows other use cases). Unfortunately I don't see an easy way to do it:

• A simple operational semantics would be a naive "taint tracking" model where bytes that were uninit and got frozen are still recorded as being "tainted" by uninit-ness, and this is propagated though essentially every operation on values, and certain operations on tainted values/memory (e.g., branching or outputting) are EB. However, this disallows use case 2 where the offending bits are masked out, and likely other use cases as well.
• To be smarter about when the "taint" of uninit-ness can be safely considered defused, one could try to do "possible values of non-det choice" reasoning like LLVM's undef (e.g., undef & 1 is either 0 or 1 and (undef & 1) >> 1 is always 0). However, this seems very hard to reason about and and possibly makes some desirable compiler optimizations illegal (w.r.t. not introducing EB).
• A more teleological definition would be that there is EB if the observable behavior of the program depends on the non-deterministic choices made by freeze operations. However, this is impossible to implement, and allows some programs that sanitizers will flag as using uninitialized memory.

View changes since the review

[RalfJung]

"program leaks secrets" is not something that you can define as a property of an AM execution, so I don't think we can have Miri detect this or call it EB. (Formally it's a hyperproperty, you need to define a notion of "public"/"secret" data and then compare two runs of the program to determine that a secret was leaked.)

A simple operational semantics would be a naive "taint tracking" model where bytes that were uninit and got frozen are still recorded as being "tainted" by uninit-ness,

I think you are inventing provenance for integers. Please, let's not.

[hanna-kruppe]

I'm well aware that "leaks secrets" can't be operationalized at AM level, and of other challenges. I'm wondering whether there is some property of an AM execution that we can define, which is somewhat related to improper use of uninitialized memory, and useful as EB: doesn't rule out any important use cases, but diagnoses some obviously buggy programs. Tools like Valgrind and MemorySanitizer are useful and already don't complain about some of the things that freeze would allow doing. It would be a shame if we had to essentially turn them off completely around any use of freeze.

Maybe the fact that freeze is opt-in is good enough to still catch all the same bugs in practice. But it's not obvious to me.

[RalfJung]

It would be a shame if we had to essentially turn them off completely around any use of freeze.

I can't think of anything better. Any way of distinguishing the result of freeze from a normal integer amounts to essentially a form of provenance on integers, and that's too big a hammer for this IMO.

[programmerjake]

imo whatever we pick should support stuff like Atomic::<(u8, u16)>::compare_exchange which needs to be able to freeze the padding bytes (probably using MaybeUninit<[u8; 4]>) and have them turn into normal bytes that don't report errors because you had to compare them in your cmpxchg loop.

[RalfJung]

Please let's not scope creep. Atomic on types with padding has a bunch of extra complications.

[programmerjake]

I'm not saying Rust should implement Atomic for types with padding, but that with freeze a user library could.

[RalfJung]

Ah I see. Yeah that may be possible, but one has to be careful in from_mut (it probably has to freeze padding).

[RalfJung]

Looks like all the first-round feedback was handled. Let's nominate this for t-lang.

[honzasp]

To be honest, I am now even less convinced that adding freeze is a good idea. If we look at the use cases from the RFC:

1. memcpy() serialization: doing this is almost always a very bad idea, because it can leak content of uninitialized memory, including secrets.
2. Masked reads: as @SkiFire13 pointed out, this can already be solved by overwriting the uninitialized padding bytes with zeros, and LLVM seems to be able to optimize this out to the exact same code as with freeze + mask.
3. C bitfields: this is an edge case that's only relevant if we access C bitfields and compile with LTO, and according to @RalfJung we can't guarantee that the freeze() function will do the right thing anyway. (And even though C bitfields may cause UB on the LLVM level, this doesn't seem to cause any problems in practice.)
4. Sparse set and other clever data structures: this is a fairly niche application, and it can be implemented safely with inline assembly or FFI.

On the other hand, we lose the property that safe Rust can never leak values of uninitialized memory without invoking UB or using inline assembly or FFI. This is not a theoretical concern, this opens a whole class of security issues.

(It might look a bit weird that I'm arguing against an RFC that I wrote, but my main aim was to resolve this question: either decide to add freeze, or decide that this operation will never be added to the language.)

[programmerjake]

another use-case for freeze: allow optimizing Simd operations -- when LLVM gains llvm.speculative.load and Rust gains a function that uses that, it can be used to load a Simd even if it might go beyond the end of the input buffer, freezing that allows you to do your simd operations without needing masking until the end, potentially running faster that way.

[RalfJung]

Possible motivations, some were already mentioned:

• SIMD where some lanes are "garbage"
• atomics on arbitrary types of small size, including types that have padding and even unions
• More general version of the previous point: Soundly deal with padding of data of arbitrary type where we cannot just manually reset the padding to 0. (E.g., implement a memcmp that's never UB, even if it may produce odd behavior for some data.)
• Data structures such as the sparse set
• As a "story" for in-memory freeze via inline asm

I feel like we have seen more use cases over the years. Cc @chorman0773 @rust-lang/opsem

None of these are Earth-shattering on their own but it adds up.

[theemathas]

Due to the existence of const generics, it is important for soundness that consteval is deterministic, even when evaluated from different compilation units. How could this be achieved with pseudorandom values? What inputs would be used as the seed?

View changes since the review

[RalfJung]

This is not the first non-determinisitc operation we allow in const-eval -- we already allow e.g. (0.0 / 0.0) (which returns a NaN with non-determinisitc sign). freeze will work the exact same way: We will pick some deterministic behavior without making guarantees about which one.

I expect we'll just fill uninit memory with zeros to start with.

[oskgo]

That's inconsistent with the paragraph being commented on. It's suggesting that freeze should return something unpredictable (not just unguaranteed).

For some pseudorandomization you can use the initialized bytes. Another thing you could do is use the type id. There might also be some suitable target dependent consts floating around.

[RalfJung]

The spec is that freeze non-deterministically picks a value.
Always picking 0 is an entirely correct implementation of that.
It is neither meaningful nor even practically possible to promise any kind of "unpredictability", whatever that should even mean. The RFC also does not use that term.

The paragraph says "it might be useful", and in Miri we'll probably randomize it as usual. But currently we don't try to do anything like that in const-eval, and doing so is orthogonal to this RFC. The details are left to the implementation, as per our usual treatment of non-determinism.

[theemathas]

I assume that we're going to do "all zeroes" for consteval in miri too?

[RalfJung]

All that matters for the RFC is that we're going to do something legal. :)

Miri usually does not change anything about what happens during const-eval (except that we set -Zextra-const-ub-checks).

[honzasp]

Thanks for the good points, I'll update the text to clarify that consteval freeze must return deterministic values.

However, I thought that it would be useful if Miri at runtime produced pseudorandom values, to help uncover bugs where the code incorrectly makes too strong an assumption about the frozen uninitialized bytes, for example if it assumes that they are always zero.

[RalfJung]

Yeah Miri should randomize (but that's a Miri QoI issue, not a normative statement).

And const-eval only needs to be deterministic in a very specific sense. We certainly don't want to promise that freezing multiple times during const-eval will return the same value, for instance. I don't think is useful to spell all this out every single time we add a new non-deterministic operations. There's nothing special about this operation in const-eval, it's just non-determinism.

[RalfJung]

Suggested change

	#### Const eval/Miri
	The `freeze` intrinsic also needs to be implemented for constant evaluation and
	inside Miri. It might be useful to replace the uninitialized bytes with
	pseudo-random values, to help uncover bugs in programs that use `freeze()`.
	#### Const eval/Miri
	The `freeze` intrinsic also needs to be implemented for constant evaluation and
	inside Miri. This will work just like the existing non-determinism e.g. for floating-point
	NaNs.

[honzasp]

I reworded the paragraph to clarify that freezing during consteval is non-deterministic, but that it's not the first such operation.

[jplatte]

Regarding use cases, would this allow implementing seqlock w/o inline assembly? I vaguely remember that it has similar problems as the mentioned Atomic<T> use case, but maybe not the exact same.

[RalfJung]

Regarding use cases, would this allow implementing seqlock w/o inline assembly? I vaguely remember that it has similar problems as the mentioned Atomic<T> use case, but maybe not the exact same.

I don't think so. The RFC to watch for that is #3301.

[programmerjake]

Regarding use cases, would this allow implementing seqlock w/o inline assembly? I vaguely remember that it has similar problems as the mentioned Atomic<T> use case, but maybe not the exact same.

I don't think so.

if you want a slower seqlock that works on arbitrary types including padding, but not including anything containing pointers, you can use freeze to do that without needing inline assembly. A demo doing that (though it uses inline assembly to implement freeze since freeze isn't implemented yet): https://rust.godbolt.org/z/zd6jTec3e

you'll want atomic bytewise memcpy for full speed seqlocks, since the compiler can do much larger loads and stores with that.

[RalfJung]

Ah right, you can do "atomic bytewise memcpy at home". There's also code somewhere that uses a derive for that, to do such a copy for a specific type. So yeah that would all work with just this RFC There are even variants of that that don't need this RFC I think.