Uninitialized memory

[carllerche]

Well, a guideline would require that an API that leaks uninitialized memory is marked as unsafe.

[comex]

I’m basically just restating what’s been already said in the thread, but -

The thing about reading uninitialized memory is that it only takes one such read to potentially get at any freed data, which breaks what is otherwise a promise that avoids a whole class of bugs in safe code - the promise that freeing data makes it go away. For some especially sensitive pieces of data, such as passwords, it is advisable to zero out the memory after use anyway, as existing C applications do, because the kernel might somehow leak that memory to other processes; but it is impractical to do this for all data which might be valuable to an attacker (which in networked applications could be just about anything - any private data stored on behalf of users), and attacks against the kernel are usually local only, so in general that attack scenario is far less scary than direct leaks by the application. Heartbleed is quite relevant whether or not that specific bug was caused by reading from an uninitialized buffer: it is an example of how dangerous information disclosure can be even if it’s not in the traditional ‘arbitrary code execution’ vulnerability category.

I don’t think the overhead of stabilizing a buffer API post 1.0, and temporary slowdown before that, is very significant compared to that risk. I agree that a narrow precedent would be best, but I can’t actually think of any scenarios, where uninitialized memory access is truly required to avoid a significant usability or performance overhead, that aren’t already highly unsafe; can anyone else?

[bascule]

Just wanted to +1 these comments by comex:

The thing about reading uninitialized memory is that it only takes one such read to potentially get at any freed data, which breaks what is otherwise a promise that avoids a whole class of bugs in safe code

I don’t think the overhead of stabilizing a buffer API post 1.0, and temporary slowdown before that, is very significant compared to that risk.

I strongly agree with these sentiments (and as a separate issue, Rust should definitely stabilize the volatile_set_memory intrinsic!). Reading uninitialized memory poses a huge security danger, and I’d hate to expose that danger to the safe subset of Rust “for speed” if there are better alternatives which can still provide safety.

The buffer traits proposed by @carllerche seem like the best path forward to me. They emphasize safety while still providing a good foundation for performance in the future.

[llogiq]

So to sum this up, we have three possible solutions:

1. Leave things as they are. The security minded folks gnaw their teeth, but what’s the worst that could happen between consenting adult implementors? Performance would be good, but valgrind may show strange things.

2. Zero out all memory on allocation (or perhaps at program start and on drop). This would imply a serious performance hit, but keep the secutiry folks happy. However, Rust is about zero-cost abstraction, so I’m quite sure this is not the right way.

3. Leave it to the type system to figure it out (e.g. wrap to disallow reads). No performance hit, no valid insecure code (as long as the library defines the right signature), benefits in other areas (e.g. OpenGL).

I for one am inclined to give my vote to option 3, but I don’t know what it’s implementation would entail.

[tbu]

consenting adult implementors

What could possibly happen between consenting adult implementors of something in C? This is an area where Rust tries to improve.

[llogiq]

I may have missed putting a <sarcasm> tag somewhere.

Note that option 3 (which is basically what reem proposed) would shift the responsibility for keeping the buffer write-only to the caller (because it would need to wrap the buffer in a write-only type before handing it to the callee).

Why? I don’t think it is feasible (as of now) to fully implement this scheme for all allocations. This would mean that allocated but uninitialized memory would be WriteOnly until initialization. It would then need to automatically change its type to ReadWrite upon writing to it. There are fairly good heuristics to deal with a large range of this (e.g. see how FindBugs tracks NullPointerExceptions in Java code), but the problem remains unsolvable for the general case. Also things like entropy pools (which are, admittedly, not that common) would then need another escape hatch.

Oh, another thing regarding safety: While it is true that reading from uninitialized memory could potentially be risky, not cleaning security-relevant data from memory is the real culprit. If you have a scenario where an attacker is already running code within your process, the compiler cannot keep them from combing through the heap.

[nikomatsakis]

Zero out all memory on allocation (or perhaps at program start and on drop). This would imply a serious performance hit, but keep the secutiry folks happy. However, Rust is about zero-cost abstraction, so I’m quite sure this is not the right way.

This is not about zeroing all memory on allocation. Most of the time, we immediately initialize memory after allocating it, and zeroing is not necessary. It’s only in the case of the helper fn read_to_end and similar patterns, where we want to allocate a buffer and then gradually write into it, where problems arise.

[vojtechkral]

Right, that’s what I was talking about. Probably security-sensitive code would want to practice security in depth, meaning that they would want to use ZeroVec and not have safe code being given access to uninitialized data.

Sidenote/FYI: for example Botan (a C++ crypto lib) has a vector type like that. Besides zeroing memory before freeing it also tries to lock the memory in RAM so that it cannot be swapped out, that’s another issue. See http://botan.randombit.net/manual/secmem.html if you’re interested in that…

[llogiq]

Ok, so the implications are less pronounced. Also this would just include one API function that allocates, but doesn’t initialize memory. Could a lint be produced that checks if the resulting memory of this API function is read before initialization?

Yes, I know that it’s intractable in general, but I’d assume that usage would occur fairly linear (those could be checked to reliably produce an error by a kind of bounds-check), and more complex usages could issue a warning.

This could be done in a backwards-compatible way, would allow more correct programs than the WriteOnly implementation (I’m not yet sure if this is a good thing) and won’t even require type system shenanigans.

Still, the OpenGL use case is a strong motivator for implementing WriteOnly anyway.

[bascule]

@vojtechkral there’s already a pretty nice library for memory protected buffers in Rust:

seb-m/tars

[llogiq]

Taking a step back and thinking about easy solutions, I came up with the idea of using a marker type + lint for this, like:

fn read(&mut self, buf: &mut [u8] + WriteOnly) -> Result<usize>;

The lint would then need to get a typed AST and err on any deref outside of an lvalue. The marker type would not even need to be generic, and could be defined now, even if we haven’t implemented the lint yet.

If that works, it would obviate the need to have a wrapper which would either be specific to one type, e.g. &mut [u8] (which would limit its function to byte buffers), or would need to be generic over the wrapped type (which, barring HKT could entail its own set of problems).

Would that be feasible or do I give Rust’s type system too much credit? Did I get the syntax right? How hard is it to get a typed AST within a lint?

Edit: No, that apparently does not work, because we cannot add trait bounds to concrete types. Too bad. It would have been a cool possibility.

[tbu]

This also doesn’t work because it’d have to check that the return value matches the number of bytes written.

[llogiq]

Then a wrapper trait that ensures the invariants (perhaps even by API design) would be the right solution. The interface could ensure that the right thing is done.

// would need to keep a pointer to the current head, could also do len-check assertions
fn append(data ; &[u8], bytes : usize)

// get the bytes written
fn bytes_written()

If needed, a Deref / DerefMut implementation could use asserts to ensure that only already written bytes are deref’d.

For the OpenGL use case, perhaps some struct could implement DerefMut, but not Deref? Otherwise, a lint could check that the struct is not Deref’d.

[gnzlbg]

I want to contribute a real world example of reading uninitialized data from multiple implementations of the MPI standard in C.

On initialization, MPI processes spawn other processes. Before spawning the children procs, each process allocates a fixed-size buffer of run-time size (same for all processes). Without going into details, this buffer is larger than the amount of data to be sent and contains the bounds of the data actually sent. The process fills it partially with data, and sends it over the network, including the uninitialized data. This is faster than just sending the data that is initialized. On the receiver process end, the bounds are read, and only the initialized part of the buffer is read.

Two things happen. On the reader process, valgrind complains on the read of uninitialized memory for sending it over the network. On the receiver end, valgrind does not complain if the child process reads out of bounds, because the out-of-bounds part of the buffer has been rightfully initialized, albeit with garbage. valgrind, of course, doesn’t know that.

A lot of people have argued about zeroing the buffer for removing this spurious valgrind error (on the sender end). It was, however, profiled than zeroing the buffer had a non-acceptable cost, and valgrind suppression files are provided instead.

[DaGenix]

This seems kinda related to https://github.com/rust-lang/rfcs/issues/926, at least in the respect that unsafe code that relies on safe code doing the right thing can result in memory safety violations.

[rkjnsn]

I agree that we should try to prevent reading from uninitialized memory in safe code, and we should leverage the type system to do so.

I want to point out that &out is not what we want, here (read would be required to fully initialize the buffer before returning). Also, the OpenGL example and the read_to_end example want different things. The same solution will not work for both. read_to_end needs to ensure that a fully safe read doesn’t look at any uninitialized bytes from the buffer (reading bytes that have already been written is theoretically fine, but probably not necessary), writes data into the buffer sequentially, and correctly returns the number of bytes written to the buffer (otherwise uninitialized bytes could end up in the final vector).

It seems like one would want something like this (modulo optimizations):

struct OutBuf<'a> {
    buf: &'a mut[u8],
    len: usize,
}

impl<'a> OutBuf<'a> {
    fn push(&mut self, byte: u8) { self.buf[self.len] = byte; self.len += 1; }
    fn push_all(&mut self, slice: &[u8]) { for &byte in slice {self.push(byte);} }
    fn capacity(&mut self)->usize { self.buf.len() }
    fn len(&mut self)->usize { self.len }

    unsafe fn as_mut_ptr(&mut self)->*mut u8 { self.buf.as_mut_ptr() }
    unsafe fn set_len(&mut self, len: usize) { self.len = len; }
}

For (my understanding of) the OpenGL example, you want something very different: code can write anywhere in the buffer, it can never read from the buffer, and there’s no need to record how many bytes have been written.

[eddyb]

For what it’s worth, I would expect a function to be able to take &out [T], initialize part of it, and return a (&mut [T], &out [T]) pair representing the initialized and uninitialized parts… Except it wouldn’t be obvious that they form a continuous slice, so you still need something like Vec to properly manage a buffer.

Hmm, my &move T design (which isn’t really public anywhere because it’s too complex to describe or finalize easily. it has also fluctuated a lot) allows for some generalization:

&'a move T = Own<T, Stack<'a>>
Box<T> = Own<T, Heap>
Box<T, A> = Own<T, A>
Vec<T, A> = Own<[T], WithCapacity<A>>
Vec<T, Stack<'a>> = Own<[T], WithCapacity<Stack<'a>>>

That last type, a “stack vector” with fixed capacity (but really, it could be borrowed from anywhere, including a &'a mut Vec) is what OutBuf seems to be aiming for. It should be possible to create one of those from an &'a out [T] (which I like to call Slot<[T], Stack<'a>>) - assuming the respective language features were in place.

[DanielKeep]

&'a move T = Own<T, Stack<'a>>

This feels related to some thoughts I’ve had on enabling garbage collection in Rust. The current big sticking point in my thinking is how to handle write barriers. The best I’ve been able to come up with is to extend lifetimes to also include knowledge of which “arena” the backing memory comes from. So &'a move T would effectively be &'(b, Stack) move T, with the full tuple being implied but not directly exposed in most cases.

I bring this up only as a possibly related point of interest.

[gereeter]

One point in favor of banning uninitialized reads in safe code that seems to have been overlooked is that, contrary to what many people have said, it can result in undefined behavior, according to the semantics LLVM defines.

The LLVM language reference explicitly states that for some read operation R,

…if there is no write to the same byte that happens before [R], [R] returns undef for that byte.

Unfortunately, although this read is not undefined behavior itself, the return of undef can cause later, perfectly safe code to run undefined behavior. For example, the following function is perfectly safe under Rust semantics:

fn weird(x: u8) {
    if x != x {
        // Do something horribly unsafe and undefined here
    }
}

However, if weird is fed undef, the result of an uninitialized read, then the code in the conditional may be run. This is because, as explained in another section of the reference,

an undef “variable” can arbitrarily change its value over its “live range”.

[aturon]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.