10
"AI" audit of rust stdlib (github.com)
submitted 6 months ago by to c/rust@programming.dev
8 comments fedilink hide all child comments
 

I checked some of the issues out, and this looks legit. Although a good chuck relates to obscure platform abstractions.

you are viewing a single comment's thread
view the rest of the comments
[–] 5 points 6 months ago* (last edited 6 months ago) (7 children)

Can someone explain how/confirm/deny that the proposed fix for this issue actually solves the problem? I think I understand the problem statement but I *fail to see how constructing a Some(&...) is in any way different than using iter::slice in terms of falsely guaranteeing immutability of user-space-backed memory.

  • permalink
  • fedilink
  • source
  • hideshow 7 child comments
    •
  • [–] 6 points 6 months ago* (1 child)

    I am not an expert but I think the actual problem is in the slice::iter first getting an immutable reference to the underlying data with self.0.next() before mapping it into a UserRef<T> which is instant UB because a &T guarantees exclusivity while UserRef<T> explicitly states in its contract that it does not guarantee anything like that, even if mutably borrowed.

    So in essence the problem, as I understood it, is in the step from slice -> reference to value -> User ref of value with the italic part being illegal.

  • permalink
  • fedilink
  • source
  • parent
  • hideshow 1 child comment
    •
  • [–] 1 point 6 months ago (4 children)

    I'm too noob to understand it all but the way I see it, the core is not about using Some instead of iter::slice but what is inside.

    I understand this all is in context of Intel Software Guard Extensions about which I know nothing at all, and "UserRef states that userspace may always write to backing memory at any time, and that even &mut UserRef is not exclusive". So it seems that the design of UserRef is basically against Rust principles ("userspace may always write to backing memory"). So when the iterator is just a wrapper over slice::Iter, the code in next() relies on Rust's principles which don't align with how UserRef is supposed to behave. The proposal is to walk over ptr "manually" and return it (cast to UserRef) instead of constructing new UserRef from what slice::Iter would return in that next().map(|e| UserRef::from_ptr(e)).

    But does

    +pub struct Iter<'a, T: 'a + UserSafe> {
+    ptr: *const T,

    really mean that one can happily cast that ptr to UserRef?
    What is PhantomData and how/why using it makes any difference, when it's not being changed during iteration?
    Is the UserRefdesign sound or was only a workaround some problem?

  • permalink
  • fedilink
  • source
  • parent
  • hideshow 4 child comments
    •
  • [–] [S] 4 points 6 months ago

    The new Iter struct basically mirrors slice's Iter, but with poiners/pointer arithmetic used for everything (the new next() impl). PhantomData is used in both to track the lifetime of the data pointer.

  • permalink
  • fedilink
  • source
  • parent
  • [–] 2 points 6 months ago (2 children)

    Regarding your question about the cast from const* T to UserRef<T>:

    This should be legal as long as UserRef<T> is a #[repr(transparent)] wrapper over a const* T and does most of the magic through its implementation.

    There are some other requirements iirc, but I am not entirely sure about them and some or all of them should be given here automatically through the architecture and/or a proper smart pointer implementation as a transparent wrapper...

  • permalink
  • fedilink
  • source
  • parent
  • hideshow 2 child comments
    •
  • [–] 1 point 6 months ago (1 child)

    Oh, a bit like inheritance in pure C? Where as long as inheriting struct puts the parent struct as first field, you can pass it to functions that use parent struct because the memory layout rules ensure that this part of memory will be exactly the same in both?

  • permalink
  • fedilink
  • source
  • parent
  • hideshow 1 child comment
    •
  • [–] 2 points 6 months ago* (last edited 6 months ago)

    Yes, except rust has even stricter requirements - namely that your struct must only contain the "inherited" field and you still need to tell the rust compiler to use the special #[repr(transparent)]!

    This is because the compiler is allowed - under "normal" representation rules - to rearrange basically everything about the memory layout of a struct to better suit its needs. And as far as I know this includes rearranging the location of a field which is arbitrarily deep inside other structures in your struct! As such

    struct A {
  foo: u8,
  bar: u8,
}

struct B {
  test: u8,
  nested: A,
}

    could theoretically be laid out as bar test foo - as opposed to e.g. foo bar for just A - in memory if the compiler determined that accessing bar at the start of the struct was overall the "best".

    If, on the other hand, you use #[repr(c)] you get exactly what you just said, although the direct casting may or may not be undefined still. (I currently do not remember the relevant parts of the nomicon or other treaties I have read about this... I really need to get back into programming rust at some point!)

    Quick edit: Expanded the possible memory layout of just the struct A.

  • permalink
  • fedilink
  • source
  • parent