Rust needs a safe abstraction over uninitialized memory

Isn't that an out-of-bounds read? That's not something that freeze can solve. Or do you somehow know the buffer is big enough?

You could use a masked load, telling the compiler which bytes to load and which to ignore. Then you never get any uninit bytes. Not sure if there's a stable API surface for it; the underlying intrinsic is simd_masked_load.

We know the buffer is big enough, it's just not initialized.

Masked loads are significantly more expensive, not cross-platform (e.g. this LZ decoder is supposed to work with just SSE2 or Arm NEON, which doesn't have any masked loads), and completely unnecessary here as the read uninit bytes will later be overwritten or otherwise ignored.

1 Like

simd_masked_load is cross-platform. I don't know what it does on SSE2.

We could have a weaker version of masked loads that needs no special hardware support, while maintaining a proper high-level abstraction: a masked load where the entire vector is loaded (so it must be fully in-bounds), but some lanes are masked off, and only the remaining lanes need to be initialized. That avoids unnecessarily throwing over board foundational concepts of Rust.

I said masked loads aren't cross-platform, not that simd_masked_load itself is. It has to be emulated on platforms that don't have native masked loads, which is expensive. Hell, even on platforms that have masked loads, they're still expensive.

We could have a weaker version of masked loads that needs no special hardware support, while maintaining a proper high-level abstraction: a masked load where the entire vector is loaded (so it must be fully in-bounds), but some lanes are masked off, and only the remaining lanes need to be initialized.

This still would add extra overhead to my example for:

  1. Computing the length of the masked portion.
  2. Expanding this length to a mask.
  3. Masking out the uninitialized elements.

All of which is completely unnecessary.

Furthermore, the implementation of this operator itself would once again require MaybeUninit::freeze if not directly lowered into a (set of) LLVM intrinsics. Why not give the more generally useful tool instead of this very limited masked load?

1 Like

It would be directly lowered to a sequence of LLVM operations, like most intrinsics are.

Because that tool pokes a hole right through the Rust memory model with global ramifications for the entire language, and has some undesirable consequences that have been extensively discussed in the ~dozen of threads about freeze. Exposing it is a last-resort-only option.

3 Likes

@RalfJung To make this overhead concrete, consider the difference in these two implementations:

#![feature(portable_simd)]
#![feature(maybe_uninit_slice)]

use std::simd::*;
use std::mem::MaybeUninit;


#[unsafe(no_mangle)]
pub unsafe fn load_freeze(
    data: &[MaybeUninit<u8>; 16]
) -> [u8; 16] {
    // UB: we don't have a freeze.
    let tmp = *data;
    unsafe { core::mem::transmute(tmp) }
}

#[unsafe(no_mangle)]
pub unsafe fn load_masked(
    data: &[MaybeUninit<u8>; 16],
    valid_len: usize,
) -> [u8; 16] {
    unsafe {
        core::intrinsics::assume(valid_len <= 16);
        let valid = data.get_unchecked(..valid_len).assume_init_ref();
        u8x16::load_or_default(valid).to_array()
    }
}

We see the following output on SSE2:

load_freeze:
        mov     rax, rdi
        movups  xmm0, xmmword ptr [rsi]
        movups  xmmword ptr [rdi], xmm0
        ret

load_masked:
        mov     rax, rdi
        movd    xmm0, edx
        punpcklbw       xmm0, xmm0
        pshuflw xmm0, xmm0, 0
        pshufd  xmm0, xmm0, 0
        movdqa  xmm1, xmmword ptr [rip + .LCPI1_0]
        pmaxub  xmm1, xmm0
        pcmpeqb xmm1, xmm0
        movdqu  xmm0, xmmword ptr [rsi]
        pand    xmm0, xmm1
        movdqu  xmmword ptr [rdi], xmm0
        ret

This would simply not be acceptable overhead in my LZ decoder.

1 Like

I've read quite a few of these threads and didn't come across (or at least didn't recognize as such) any such global ramifications/undesirable consequences for by-value freeze. Do you have a reference to such a global ramification or undesirable consequence which is:

  1. Relevant to by-value freeze (MaybeUninit<T> -> MaybeUninit<T>). I agree that by-ref freeze is highly problematic with MADV_FREE existing and other optimizations.
  2. Not directly or indirectly a concern stemming from "reading uninit memory is inherently bad", e.g. heartbleed related arguments. These arguments simply make freeze bad by definition, and they could be trivially dispelled by marking freeze as unsafe, reminding you of the potential security consequences of leaking (indirectly or otherwise) uninit memory.

I said this upthread but it was buried in a footnote:

I do not agree with the suggestion that it's automatically a bug in the language to provide [freeze] even for non-pointer types. In the conventional multiprocessing model, there are no true security boundaries within a process, and it is the operating system's responsibility to clear sensitive data out of memory newly allocated to each process. In an environment with finer-grained memory capabilities, like what would be possible with CHERI, some of that responsibility moves to the in-process allocator -- free needs to wipe the "this is a capability" bit on each word of storage, for instance -- but I still don't buy the argument that allowing a value to leak from one part of a process to another is unacceptable.

I could be convinced otherwise with concrete examples of problems, for scalar non-pointer types, that cannot be fixed by introducing (more) privilege separation. I do not recall any such example from previous threads (but I could have missed one).

3 Likes

This is the main global ramification — to justify freeze being unsafe, we have to say that leaking the contents of freezing uninitialized bytes to arbitrary safe code is considered unsafe. Whether this is actually a reasonable property is debatable (see e.g. zackw's position), but it's currently the case that doing such is impossible (UB), and this also makes utilizing sanitizers such as MSan much more practical than it would be if freeze is something your dependencies are freely allowed to do.

(C.f. that atomic fences are already problematic for TSan, and the std Arc code actually replaces its use of fences with an additional load/store as needed to work properly with thread sanitization.)

I think an Option may be a good solution, since it is actually a tagged maybe_uninit, which uses Some to confirm init None to confirm uninit.

The actual problem with a safe freeze for me is that it completely breaks the parse, don't validate pattern, which makes it possible to create types that ensures any invariant by marking its fields private and constructing it only through specific, carefully planned APIs that assert all required invariants. By following this pattern, we can guarantee that as long as we stick to safe code we will never encounter a value that violates one of its invariants. But with safe freeze and enough luck (or maybe by scattering junk memory in a certain pattern), we can manufacture any value of a given type out of thin air. (Unsafe code already trivially breaks the parse-don't-validate thing, so we can only say that safe Rust respects it)

So, if we had safe freeze, this means that trusting invariants in safe code now requires global analysis (we must verify that freeze was not involved in the manufacturing the value itself or any input to its constructor functions, recursively), which is super uncool and also unfeasible. Marking freeze as unsafe means that whenever you call it, you are telling the compiler "trust me, I got this right" (so much that I think that trustme { .. } blocks would be much better than unsafe { .. }). It wouldn't lead to memory unsafety / UB, but would lead to breaking invariants that otherwise can't be already broken with safe code. So a safe freeze materially reduce what guarantees we have about safe code, and really shouldn't be done.

But of course it doesn't cause UB so it's hard to justify it being marked unsafe. Maybe one way to think about it is that rather than unsafe fn freeze, we should have something like unsafe(invariants) fn freeze (or something like that; it has been proposed multiple times before, but I think that past proposals didn't have such a concrete use case). So freeze is not unsafe because it causes UB, it's unsafe(invariants) because it's a escape hatch that, if misused, can subvert guarantees that are generally afforded to safe code.

(We could even imagine that, during the leakapocalypse, rather than turning unsafe fn forget into a safe function - justified by noting that it doesn't lead to actual UB - we would have something like unsafe(leak) fn forget or something. Well as long as Rc and Arc were dealt with as well)

I probably just don't understand the issue at hand, but isn't this just a limitation of thread sanitizer itself, rather than evidence that atomic fences are somewhat bad? (a counterpoint is that Zig removed atomic fences due to this very issue which means that at least some people think they are bad)

But anyway - taking a look at the Arc code, if swapping implementations with #[cfg] just to appease the sanitizer overlords is deemed acceptable, then why can't this be done with freeze as well?

We would need some compiler-implemented notion of a "default value" that arbitrarily picks a value of a given type (or panics, or fail to compile, for uninhabited types). This might be just a zeroed value, for types without a niche at zero - or the first valid bit pattern, skipping niches (so maybe the default value of a NonZeroUsize is 1 or something). Then freeze does it usual thing when called outside of sanitizerss, but call this default thing when running under MSan. Which of course doesn't perform an actual freeze at all, but Arc isn't doing an atomic fence when running under TSan either.

2 Likes

RFC: Never allow reads from uninitialized memory in safe Rust by aturon · Pull Request #837 · rust-lang/rfcs · GitHub lays down the issue pretty well.

No, that does not work. unsafe has one purpose and one purpose only: to guard against UB. If we provide freeze, it is by definition not UB to read some uninitialized u32 and freeze it. We don't use unsafe as a lint; it is considered an anti-pattern to mark a function as unsafe that cannot cause UB.

Even if we wanted to change this and somehow extend the scope of unsafe to include freeze, we'd need a clear (in the mathematical sense) definition of which uses of freeze need to be unsafe or not. It is not quite clear what that would look like, but it would likely be a form of non-interference. That's a qualitatively different property from UB freedom, so even if we found a clear definition we would now have made the life of everyone who systematically reasons about unsafe infinitely harder.

Literally the entire point of Rust is that we have very fine-grained reasoning about which part of the program can do what to which parts of memory when. To suggest that we should revert back to the simplistic "no boundaries within a process" is to suggest to throw most of the Rust language concepts out of the window.

That's impossible. freeze is specified as "preserve original value if memory it is initialized, otherwise pick one non-deterministically". How do you suggest the sanitizer-friendly fallback implementation checks whether the original value is initialized or not?

This needs dedicated sanitizer support so that we can tell the sanitizer that a freeze happened, and it can update its shadow state accordingly.

Arc is doing some other correct thing instead. The fence gets replaced by an acquire load. However, freeze cannot be replaced by an alternative in the same way.

3 Likes

Don't worry… nobody is proposing a safe MaybeUninit<T> -> T function for arbitrary T. That's quite impossible for the reason you stated.

If there were a safe freeze, it would have to be either limited to specific types or typed as MaybeUninit<T> -> MaybeUninit<T>.

5 Likes

Yeah freeze is awful. The problem is that, despite all issues and all bad things, sometimes it may be necessary to read uninit memory, and my understanding is that MaybeUninit can't be used for that even in unsafe code (calling MaybeUninit::assume_init on uninitialized memory is UB), nor can ptr::read or other unsafe APIs. So I guess that people must resort to assembly for that (reading uninit memory in asm is fine).

Anyway this RFC was closed. What was ultimately the result of this discussion? Is exposing uninit memory to safe code considered UB?

And what about something weaker: is exposing uninit memory to unsafe code UB, even if don't let any safe code take a look? Is there anything we can do with uninit memory, other than write to it?

But also I wanted to discuss whether this is actually impossible,

Once upon a time, Drop types had a new, hidden field that tracked whether the value should be dropped (a drop flag). This made them bigger and was generally awful, so eventually drop flags were removed from types themselves and moved to the stack, tracking whether local variables should be dropped (I can't readily find a source but there's a discussion here)

But before drop flags were removed from types, it was considered acceptable that sometimes Rust would insert a hidden field into a type just to track that it did some stuff. So when compiling with sanitizer support, rustc could add such a hidden field to track whether a value is initialized, just for purposes of freeze. (this would incur a massive overhead and thus not really be a practical solution, because any and every write would also touch this "init flag")

Of course this would miss initialization that is done by something external to the program (such as by the OS, or by another userland program through IPC, or even by the hardware). Not sure whether this is relevant.

(That's probably a better solution)

How would a safe MaybeUninit<T> -> MaybeUninit<T> freeze work? Wouldn't I just be able to immediately call .assume_init() afterwards, since the value is now frozen (and thus considered initialized)?

I think this isn't actually hard, at least for MSan.

MSan already has some handling for freeze instructions, so it might just come for free.

Alternately, there is a function __msan_unpoison(ptr, size) which you can call to mark a region of memory as initialized.

Valgrind, however, would be hard. Valgrind has its own runtime call similar to __msan_unpoison; in Valgrind's case it's a C macro that apparently expands to some magic inline assembly sequence. But unlike MSan, Valgrind's presence isn't necessarily known at build time. You could stick the magic sequence in every freeze operation in every build, but that sounds completely unacceptable for performance with the kinds of fine-grained freeze usage being discussed. Instead it would probably have to be an operation separate from freeze, that you would manually apply to an entire uninitialized buffer.

2 Likes

Yes, it would be well-defined to immediately call assume_init. But since assume_init is unsafe, you would still be prevented from conjuring T out of thin air with purely safe code. Honestly this design seems unnecessarily convoluted to me, but I mentioned it since it came up earlier in this thread.

1 Like

I've read this issue before, and I still don't see anything that qualifies as "a global ramification or undesirable consequence" you mentioned earlier:

Allow me to re-ask my question without the red herring on whether or not marking freeze as unsafe would satisfy the skeptics, or what that would mean exactly.

Do you have a reference to such a global ramification or undesirable consequence which is:

  1. Relevant to by-value freeze (MaybeUninit<T> -> MaybeUninit<T>). I agree that by-ref freeze is highly problematic with MADV_FREE existing and other optimizations.
  2. Not directly or indirectly a concern stemming from "using uninit memory is potentially leaks sensitive information", e.g. heartbleed related security arguments. These arguments simply make freeze bad by definition, because it could leak information, regardless of how it's used.

@RalfJung If your argument really just comes down to "using uninit memory is potentially leaks sensitive information", then I can respect that at least, but I'd like you to state it as such without hiding behind handwavy "global ramifications or undesirable consequences". If you really do have another technical argument not covered by my two conditions above, I'd love to hear it.

For me personally, assuming there isn't some other technical argument against freeze, I would be satisfied by a strongly worded comment on freeze noting how it's a potential security issue. Furthermore, really sensitive information like encryption keys and passwords should already be wiped from memory before calling free regardless of freeze existing for a variety of reasons.

2 Likes

The overhead of tracking, for every single byte, whether it is initialized, would be orders of magnitude higher than the drop flag Rust used to have. I don't think anyone would consider this acceptable.

No consensus has been reached. I am bringing this up to explain why adding freeze is not an "obvious solution" by any means, since not everyone here seemed to be aware of all the background that is part of this discussion.

There's also an RFC proposing to add freeze:

It has not reaches consensus thus far, either.

The RFC I linked spells it out, but sure, I can repeat it.

Before freeze: every UB-free Rust program is guaranteed to never depend on the contents of uninitialized memory.

After freeze: crickets

So, there's a global property of the language, and a useful one, just gone entirely the moment we add freeze.

You seem to be aware of this consequence, and are trying to exclude it from discussion with your point (2), but I don't accept that we should exclude it from discussion.

3 Likes

I'm not trying to exclude it from the discussion, I'm trying to clarify what it is we're exactly discussing. You alluded to vague further ramifications and consequences when in fact the issue of by-value freeze is very clear and up-front: you can read uninitialized memory, for better or for worse.

I just wanted to make sure there wasn't some hidden gotcha I was missing like "but now this perfectly valid code optimizes worse because we have to consider by-value freeze existing". Thank you for confirming that this isn't what you were alluding to, and you were in fact simply stating that freeze existing lets you read uninitialized memory and this makes you uncomfortable outright. Again, I can respect that, but I do want to know what it is I'm discussing against instead of unclear shadowy nasal demons.

1 Like