Storing an object as &Header, but reading the data past the end of the header

[thomcc]

This is related to #2 but the read is not out of bounds of the allocation, not being written to by other threads, not the bytes of a &mut Blah, etc. That is to say, really the code is trying to model a dynamically sized type, that for one reason or another does not support (Note that ther are a number of custom DST proposals).

So, I heard that it was UB for you to have a &T and read outside the bounds of that T, even if conceptually it's a totally in-bounds read. E.g. T here may be a ZST, or it may be a header after which a trailing array is expected, or standing that sits at the head of a trailing array, or it may be a struct that's the common shared fields of some set of other struct... These are pretty common in unsafe code as it's a pattern which is both legal and useful in C and C++.

It's pretty common in Rust too:

• It's not unheard of in C apis to use a #[repr(C)] struct Foo { _priv: [u8; 0] }, as this is what bindgen uses. Some of these APIs then go on use &Foo in the Rust code. (This is essentially a workaround for a lack of a stable extern Type). This code doesn't read the data, so the only issue would be if we told LLVM it could assume things about the pointer that turn out to be untrue in a situation like cross-lang LTO, probably.

• Similarly, I've seen other FFI code that used a struct CStr([u8; 0]) for a similar purpose — as a version of std::ffi::CStr that you can actually pass to C directly. (I even almost did this for ffi_support::FfiStr, but went with a pointer inside so I could easily check for code passing in null).

• bitvec has a BitSlice type which acts a lot like a slice that magically has bit-level indexing. Internally it's something like struct BitSlice { _mem: [()] } which lets it behave like an unsized type, The "pointer" and length are both specially encoded values that contain both the actual pointer/length as well as bit-level offsets for tracking where withing byte things are. There are a lot of reasons this might be illegal, but I had not thought mem::size_of_val returning the wrong value was the actual one.

• anyhow::Error internally wraps a Box<ErrorImpl<()>>, where ErrorImpl<T> contains a vtable, a backtrace, and then the T. ErrorImpl<()> is used as it behaves as the "common header" for real ErrorImpl values. On construction, Box<ErrorImpl<T>> is converted to Box<ErrorImpl<()>>, when stored in the Error.

  Whenever a method is called that needs to delegate to the vtable, the Box<ErrorImpl<()>> is converted into the right pointer type for the vtable function (one of &ErrorImpl<()>, &mut ErrorImpl<()>, Box<ErrorImpl<()>>) which is called with that pointer. The first thing the vtable function generally does is convert the reference to e.g. &ErrorImpl<T>, example: https://github.com/dtolnay/anyhow/blob/99c982128458fecb8d1d7aff9478dd77dac0ee3b/src/error.rs#L538-L545. (I had always kind of thought it wasn't okay to use Box<T> here, but I'm surprised that stuff like &ErrorImpl<()> to &ErrorImpl<RealType> isn't okay either).

• wio-rs contains VariableSizedBox which provides this pattern in a library form, and IIUC is mostly intended for the flexible-array-member case. The API attempts to launder pointers to the object, which is... very non-obvious. It seems like it plausibly avoids the issue here, though, but it's insanely subtle, and if this is the recommended pattern, I suspect it will need a very good nomicon entry. https://github.com/retep998/wio-rs/blob/9bf021178b2d02485f1bd35e6cff41bf52d4a9a2/src/vsb.rs#L98-L113

• I do something similar in arcstr, where there's a header and a variable length segment that trails it. I avoided issues here by luck, as I took great care to avoid ever putting the inner type behind a reference. This was lucky since I wasn't aware of this at all, and did it for other reasons. This was painful as it required field hard-coding offsets.

• This isn't to say anything of the numerous C or C++ apis which expose polymorphism in this way — In c++ this is how single non-virtual inheritance is represented, so it's especially common, although it was common in C too. Additionally, C code with a flexible array member is in tons of places, and not just windows APIs.

This is just a few off the top — there's a lot of unsafe code that does this. Personally, I had thought it was allowed so long as you don't go past the actual bounds of the allocation, it makes some sense that it's not though, unfortunately. (Somehow, I don't think I've ever had miri trouble me about it, but it's seeming like it's just because of luck && coincidence more than anything else).

Anyway, I think if this is UB we should start being way more vocal about it, because it's a totally legal pattern in C and C++, and common.

[thomcc]

Anecdotally, I think why I thought this was allowed is it mirror how some other functionality of references behave. In particular converting &'a T => &'static T is legal, so long as you only ever use the T within its actual lifetime. (And in the case of &mut, so long as the &mut really is granting exclusive access).

Additionally, the fact that in many other cases we're allowed to "pun" memory and have it just work the way we expect gave me a false sense of security here.

[burdges]

At least if using C FFIs then you want extern type for this. See rust-lang/rfcs#1861
rust-lang/rfcs#2255 rust-lang/rfcs#2984

[RalfJung]

As you noted, this looks like a duplicate of #134.

Note that all of these things are allowed to raw pointers. So all of these APIs that you describe can be implemented, as long as only raw pointer and no references are used.

But for references, this would be a serious problem as it breaks a very powerful optimization:

fn optimized(x: &mut i32, f: impl FnOnce()) {
  *x = 5; // This write is redundant and can be removed, according to Stacked Borrows (under unwind=abort)
  f();
  *x = 6;
}

If we allow reads outside the bounds of the given reference, we can no longer do this optimization!

fn print_neighbor(x: &mut i32) {
  // use some unsafe code to read the memory at `x - 4` and print that to stdout
}

fn counterexample() {
  let mut pair = (0, 0);
  optimized(&mut pair.0, || print_neighbor(&mut pair.1));
}

The optimization changes what gets printed to stdout. Optimizations that change program behavior are illegal, so we cannot do this optimization -- except if the program has UB. That's why it is very important that the above program has UB.

(Things get even worse if we also allow writes outside the range indicated by the type. At that point I do not know how to still have any useful optimizations.)

[Diggsey]

Hmm, I have code which does this because using raw pointers everywhere results in really horrible code. It implements a "thin vec" type, where the length and capacity are stored inside the allocation.

#[repr(C)]
#[repr(align(4))]
struct Header {
    len: usize,
    cap: usize,
}

(The items are stored inline following the header)

All the internal methods are implemented on the Header type, and take &self or &mut self, and then use pointer arthmatic from self to access the items known to follow.

This is one of several "thin" wrapper types that each have their own header type. Rewriting them all to use raw pointers would result in very messy and hard to read code, not to mention less safe (since atm the unsafe parts can be fairly contained).

This Header type can never exist on the stack or as part of another data-structure, so perhaps there's some way to exploit that? A couple of ideas spring to mind:

1. Limit the scope of UB to cases where the struct is on the stack or is contained within another type. This leaves the compiler free to optimize the case you mentioned above.

2. Allow types to opt-in to being unsized, and prohibit these optimizations on these types.

[Diggsey]

3. Have some kind of provenance rule similar to the &mut/& provenance for raw pointers discussed in Differences between *const T and *mut T. Initially *const T pointers are forever read-only? #257, but for layout, where all that matters is how the original pointer was obtained.

   So, the initial pointer returned from alloc() is allowed to access anywhere in the allocation. References derived directly from that pointer are also allowed to access anywhere, but as soon as you go through a field access, you're then constrained to the layout of that field.

[RalfJung]

One difference between my example and the desired use-cases is that not only is the pointer used "out of bounds", there also is another pointer that actually accesses those out-of-bounds parts. Maybe that could help.

Here's the rough idea: if we have a mutable reference p: &mut (i32, i32), then if we create two derived references, say &mut p.0 and &mut p.1, this all can only work out if the derived references are used in a disjoint way. Currently this is enforced by pre-determining which memory which reference may use: only the one described by its type. If we remove the type from the picture, we'd instead have to dynamically track which locations are used by both references, and then raise an error the moment those sets stop being disjoint.

I am sure interesting corner cases will show up when this gets actually implemented. But this will require a significant rework of Stacked Borrows -- basically a whole new model, based on what we learned with the first one. I certainly hope to pursue this project at some point, but unfortunately that won't happen in the near future.

[burdges]

As for implementation corner cases, there exists some consensus in the RFCs linked above that size_of_val, align_of_val, and even Box::drop all require the type's size without dereferencing, and thus should panic when passed such truly unsized types via thin pointers, including extern types.

[thomcc]

@RalfJung Yeah, I would have assumed your example was invalid, and you figured it out but it's much broader and more dodgy than what I'm asking for.

That said, your suggested solution of tracking this based on the locations the references may use sounds extremely nice and easy to reason about!

I think I'm already a huge fan, since it matches my mental model very closely — That is, in a situation like this where I have a &T and read outside of it, in reality my &T just extends to those locations I access dynamically, but still follows &T semantics (similar to how DSTs work, but obviously not supporting size_of_val). Ditto for &mut T. The way I reason about "follows &T semantics" is more or less based on memory locations (I've certainly been trying to reason about it closer to how stacked borrows is currently formulated though, ...).

Also, it feels like this model would allow #243, which is... important for memory allocators.

that won't happen in the near future.

Honestly, just the notion that there's a potential model in the distant future that fits better with the code that is out there in the wild makes me very optimistic, since I had thought stacked borrows was mostly in a final tweaking phase and major changes weren't in the cards.

I was kind of getting pretty worried about how bad the the fallout was going to be, so it makes me feel better.

(Off topic, but related to you not having time: I saw you finished your PhD recently, congrats! Hope you're able to relax at some point and manage to land somewhere nice)

[RalfJung]

Also, it feels like this model would allow #243, which is... important for memory allocators.

Ah yes, that would be nice.

I am also worried about how much the current semantics relies on computing the size of a value; if we ever get custom DST that would be a total nightmare. So something more based on "what locations is this actually used for" would also help here.

OTOH some optimizations rely on introducing extra reads/writes that were not present before; I cannot imagine how those would work without taking into account the size of a type. Basically, everything related to protectors seems to rely pretty fundamentally on saying in advance that some region of memory "belongs to" this reference. But maybe it suffices to consider these a lower bound for what the reference may do, as opposed to now where this region also serves as an upper bound.

Honestly, just the notion that there's a potential model in the distant future that fits better with the code that is out there in the wild makes me very optimistic, since I had thought stacked borrows was mostly in a final tweaking phase and major changes weren't in the cards.

More research will be needed to show if this model is indeed viable. But as far as I am concerned, Stacked Borrows is the first word in terms of (precisely worked out) aliasing models for Rust, not the last.

(Off topic, but related to you not having time: I saw you finished your PhD recently, congrats! Hope you're able to relax at some point and manage to land somewhere nice)

Thanks! :) But what is that "relax" thing you are talking about? ;)

[thomcc]

But maybe it suffices to consider these a lower bound for what the reference may do, as opposed to now where this region also serves as an upper bound.

Yes, that seems totally reasonable, and also fits with what unsafe code I've seen in the wild does/assumes (as well as my personal mental model).

It's also easy to teach/explain why a too-large reference is bad (compiler allowed to insert speculative and spurious reads/writes), whereas it seems much harder to explain a too-small one (providence, stacked borrows).

Even C tends not to have a pointer refer to a larger type than reality — although I believe it is legal for SomeUnion* to actually only be a pointer to one of the members. Using this is rare though, and Rust code which wants to emulate it can just use raw pointers.

[chorman0773]

One suggestion I have is to go with a similar idea as C++ has with pointer-interconvertibility and reachability.
C++, in std::launder, defines that a byte b is reachable from a pointer p if p points to or past-the-end of an object o, such that b is part of the object-representation of any object pointer-interconvertible with o or the immediately enclosing array thereof.
Pointer-interconvertibility is defined as an equivalence relation as follows:

• An object o is pointer-interconvertible with itself
• The first member of a standard-layout struct (which is a concept analogous to repr(C)) is pointer-interconcertible with that struct ...
• Any member of a union type is pointer-interconvertible with that union.
• An object a is pointer-interconvertible with an object b if there exists an object c such that a is pointer-interconvertible with c and c is pointer-interconvertible with b.

I'd propose something similar for references. If you have a reference r to a type T, it can access a byte b if the reference includes b, or if some preceding borrow item provides the access to b, such that the item is pointer-interconvertible with r, as follows (where pointer includes reference):

• A pointer to the first member of a repr(C) struct is pointer-interconvertible with a pointer to the struct
• A pointer to any member of a repr(C) union is pointer-interconvertible with a pointer to the union
• A pointer to the transparent (non 1-ZST) member of a repr(transparent) type is pointer-interconvertible with a pointer to the type, or if there is no such member, a pointer to any member of such a type is pointer-interconvertible with a pointer to that type (note: this is a reasonable interpretation of repr(transparent) which leaves the offsets of 1-ZST members unspecified. Another possible version could always have a pointer to any member be pointer-interconvertible with the repr(transparent) type)
• Pointers p and q are pointer-interconvertible if there can exist some pointer r, which, by a finite number of applications of these rules is pointer-interconvertible with p, and likewise pointer-interconvertible with q.
• The relation is symmetric and reflexive

One thing I'd want to ask is if this should include the "immediately enclosing array" rule.

[bjorn3]

How would impl<'a, T, const N: usize> TryFrom<&'a [T]> for &'a [T; N] be soundly implemented under those rules? This is a stable and safe conversion from a pointer to [T] which isn't a repr(C) struct or an union to a pointer to [T; N] which is repr(C) I think. And what about code like https://github.com/rust-random/rand/blob/fde4113cad99cc02ebdbb0439bbcd0d1ac5ff2d1/rand_core/src/impls.rs#L64-L69 that casts *const [T] to *const u8 to pass to copy_nonoverlapping?

[chorman0773]

How would impl<'a, T, const N: usize> TryFrom<&'a [T]> for &'a [T; N] be soundly implemented under those rules

The Slice reference sr can access size_of_val(sr) bytes.

And what about code like https://github.com/rust-random/rand/blob/fde4113cad99cc02ebdbb0439bbcd0d1ac5ff2d1/rand_core/src/impls.rs#L64-L69 that casts *const [T] to *const u8 to pass to copy_nonoverlapping

Conversion to a pointer is not considered by these rules, nor does conversion between raw pointer types.

Basically, my proposed rules would leave the provenance of referenced intact as-is, but allow for restricted extension of that provenance. The existing cases are covered by "the reference includes b"IE. r already has provenance for b (I should probably rephrase that portion of the definition to something similar to this). Any code that is currently well-defined would remain well-defined.

[RalfJung]

One suggestion I have is to go with a similar idea as C++ has with pointer-interconvertibility and reachability.

Note that the question is not about how raw ptrs interact with each other. I think this is the only place where the C++ rules could possibly be useful, but Rust is maximally permissive here currently (more permissive than C++) and I see no problem with this. The hard questions are all about cases where one side uses raw pointers and the other side uses references, and we want to maintain strong aliasing guarantees on the reference side. I don't think C++ can be much of an inspiration here since C++ has nothing close to the kind of aliasing guarantees Rust does. (C has something, namely restrict, but the spec is unfortunately not very useful, in my opinion).

[chorman0773]

No C+, does not have aliasing guarantees (beyond strict-aliasing). However, this is more of a provenance question. In this case, rust is less permissive then C++ and C, where if you have the type

struct Layout{
    struct Header h;
    std:;string name;
    std::uint64_t rest;
   };

The rules I mentioned with C++ allow you to reinterpret_cast from Header& to Layout& (provided you know that you have a reference to the header field of Layout), where as rust does not permit a similar case, as this thread is discussing. Aliasing is a different problem that would build on top of this. These rules determine when you are able to extend provenance by casts (and perhaps also transmutes), but you still have to avoid violating the aliasing restrictions (in particular, if you tried to extend provenance of a reference, it looks for a borrow item that provides the necessary access, which I presume would imply popping any item that doesn't).

[RalfJung]

However, this is more of a provenance question. In this case, rust is less permissive then C++ and C, where if you have the type

Rust is more permissive than C and C++. This is assuming you translate C/C++ pointers to Rust raw pointers.

Rust references are more restrictive, of course, but C/C++ do not really have an equivalent type. (Maybe one could make the point about C++ references? That seems unlikely though. Those references still correspond more to raw pointers in Rust than to references. In particular, they do not come with any aliasing guarantees.)

where as rust does not permit a similar case, as this thread is discussing

Rust does not permit this when mixing references (with strong aliasing guarantees) and raw pointers (without any aliasing guarantees). This question does not even come up in C++, so it is impossible to draw a parallel here.

The issues discussed in this thread are a consequence of the aliasing guarantees, so this is very much not a "different problem". The only thing in Miri that leads to a complain about here is Stacked Borrows, which is all about aliasing. If it wasn't for the aliasing rules enforced by Stacked Borrows, the &Header pattern would work just fine.

[CAD97]

One thing I just thought of: when someone goes to try out the "delayed"/"on-use" aliasing restrictions, be wary of reborrows. Reborrows happen fairly often (which is obvious for &mut since &mut isn't Copy, but less obvious for &, since it is, but matters for lifetimes), and it would be unfortunate if it were worked out that &Header could access the whole alloc.... until you passed it into a function and it is reborrowed.

I don't expect this to be an issue, but it popped into my head as a worry so I wanted to write it down.

[thomcc]

Currently, when not configured to emit #[derive(Copy, Clone)], bindgen translates the following C:

union SomeUnion {
    int32_t int32;
    double float64;
    void *ptr;
};

into something like the following Rust:

#[repr(C)]
pub struct __BindgenUnionField<T>(::core::marker::PhantomData<T>);
impl<T> __BindgenUnionField<T> {
    #[inline]
    pub const fn new() -> Self {
        __BindgenUnionField(::core::marker::PhantomData)
    }
    #[inline]
    pub unsafe fn as_ref(&self) -> &T {
        ::core::mem::transmute(self)
    }
    #[inline]
    pub unsafe fn as_mut(&mut self) -> &mut T {
        ::core::mem::transmute(self)
    }
}

#[repr(C)]
pub struct SomeUnion {
    pub int32: __BindgenUnionField<i32>,
    pub float64: __BindgenUnionField<f64>,
    pub ptr: __BindgenUnionField<*mut core::ffi::c_void>,
    pub bindgen_union_field: u64,
}

in order to emulate #[repr(C)] union. This has a few problems and ideally it wouldn't be written this way (as mentioned in rust-lang/rust#81996), but it is another example of a case where the current requirements are too strict, and people expect to be able to "widen" a &T or &mut T.

That said, this feels different in a few ways to the other examples, and my gut is that were it not common in the wild due to tooling output, it wouldn't be that important to support (especially when other alternatives exist).

[purplesyringa]

We stumbled upon a weird interaction between this issue and interior mutability:

use core::cell::Cell;

struct Header;

#[repr(C)]
struct Data {
    header: Header,
    value: Cell<i32>,
}

fn main() {
    let data = Data {
        header: Header,
        value: Cell::new(0),
    };
    dbg!(data.value.get());
    let header = &data.header;
    let data = unsafe { &*(header as *const Header as *const Data) };
    data.value.set(1);
    dbg!(data.value.get());
}

error: Undefined Behavior: write access through <3342> at alloc673[0x0] is forbidden
   --> /home/purplesyringa/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/mod.rs:872:9
    |
872 |         ptr::write(dest, src);
    |         ^^^^^^^^^^^^^^^^^^^^^ write access through <3342> at alloc673[0x0] is forbidden
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental
    = help: the accessed tag <3342> is a child of the conflicting tag <3330>
    = help: the conflicting tag <3330> has state Frozen which forbids this child write access

As far as I can see, the problem is that Header: Freeze so &data.header drops write permissions, even though a cell exists in the allocation and can be soundly accessed under TB. Adding a ZST UnsafeCell to the header makes Miri shut up.

I'm not sure if I can even classify this as a bug, but it surely is a counterintuitive interaction.

[RalfJung]

A &Header reference will be considered read-only, so mutating anything through it is highly questionable. I am not even entirely sure whether what TB does here makes sense...

[phip1611]

@purplesyringa

unsafe { &*(header as *const Header as *const Data) };

Casting &Small to &Big is always UB, if I'm not mistaken. In multiboot2 space, I solved the problem with the header pattern by never casting &Small to &Big but by always having a [u8] with that I can perform same-size casts to the corresponding types. I use &Small respectively &Header only to read the size from the header. Complicated stuff, I know.

I actually did a very comprenensive writeup including diagrams what I did there, as it cost me several days if not even weeks to get it all Miri safe:

• README with figures
• docs.rs documentation of multiboot2-common

From figure parsing-flow-specific.drawio.png, this is the relevant excerpt:

I use the &Header only to get the size for a cast, not to cast the type through a reference of &Header! I hope this helps you.

[purplesyringa]

@RalfJung IMO, TB is inconsistent here. I'd be fine with "references don't shorten provenance" (which is what TB currently does in most cases, as I understand it), and I'd be equally fine with "references do shorten provenance" (SB-style), but what's happening here is that the rules are different depending on whether Header is Freeze, which looks totally unrelated.

@phip1611 I don't believe this is always UB. It's UB under Stacked Borrows, but Tree Borrows allows this, IIUIC.

[RalfJung]

what's happening here is that the rules are different depending on whether Header is Freeze.

So TB rejects the code as written, but accepts it if you add an UnsafeCell?

That is exactly the intended behavior. If there is no UnsafeCell in the type, we also assume there is no UnsafeCell "around" it. This is important, otherwise we would have to ditch all the shared reference optimizations. If you use type erasure schemes (like accessing the full data with just the Header) mixed with references, it is your responsibility to preserve all the important type information, like UnsafeCell.

The recommended approach is to use only raw ptrs, no references. By using references mixed with raw pointers, you are opting in to "hard mode Rust".

I don't believe this is always UB. It's UB under Stacked Borrows, but Tree Borrows allows this, IIUIC.

Code accepted by TB but not by SB should be considered "not at immediate risk of miscomputation, but also not in the supported fragment, and it may be declared UB in the future".