`borrowed_with_owner`, an alternative to `owning_ref`

announcements
1

I have a crate I've been working on that is meant to be a better alternative to owning_ref, mainly by being more general in supporting arbitrary borrowed objects, not just references, without you having to resort to unsafe code. I believe it is sound, but am hoping to get some feedback to confirm or deny that. If you want to take a look or try it out, here's a link to the repo on GitHub: GitHub - mikeyhew/borrowed_with_owner: A Rust crate that lets you bundle up a borrowed object with its owner, making it `'static`

2

I've got some experience reviewing self-referencing data type crates. I like the confidence, but the first version is (almost) never sound. The more issues you fix, the more subtle the new ones will get; but if you keep at it, it can be possible to get to something that might actually be sound; so good luck ^^ (How quick and easy this is depends on the complexity of the API. I guess this case is somewhat straightforward in that at least no macros are involved, compared to e.g. crates like ouroboros.)

It's a bit later here in Europe :sweat_smile: but I couldn't skip taking the challenge immediately and finding some first soundness issues anyways. So for now, here's some nice and safe code example using your crate's API entirely improperly :wink:

#![allow(unused_assignments)]

use borrowed_with_owner::{BorrowWithLifetime, RefWithOwner};
use std::marker::PhantomData;

fn main() {
    let s = String::from("Hello World");
    let r: &str = transmute(&s[..]);
    drop(s);
    println!("{r}"); // use after free :-)
    
    let p: &usize = transmute(0_usize);
    println!("{}", p); // dereferencing a null pointer reference :-)
}

fn transmute<T, U: 'static>(t: T) -> U {
    #[repr(C)]
    enum Opt<T> {
        Some(T),
        None,
    }
    #[repr(C)]
    enum Transmute<T, U> {
        T(Opt<T>),
        U(Opt<U>),
    }
    struct Mut<U>(PhantomData<U>);
    impl<'a, U: 'a> BorrowWithLifetime<'a> for Mut<U> {
        type Borrowed = Option<&'a mut Opt<U>>;
    }
    
    let mut x = RefWithOwner::new(&()).map::<Mut<U>, _>(|_, _| None);
    let mut trans = Transmute::<T, U>::U(Opt::None);
    
    if let Transmute::U(ref mut u) = trans {
        *x.borrowed_mut() = Some(u);
    }
    
    trans = Transmute::T(Opt::Some(t));
    
    match std::mem::replace(x.borrowed_mut().take().unwrap(), Opt::None) {
        Opt::Some(u) => u,
        _ => unreachable!(),
    }
}
5 Likes
3

Thanks for the quick reply! It's getting pretty late here on the west coast of Canada, so I'm going to have to look into this more later to figure out what's going on in your transmute function, but this is exactly the kind of feedback that I'm looking for.

4

This crate is also in conflict with noalias, it needs to use AliasableDeref instead of StableDeref. Alternatively you can just always wrap the owner in AliasableBox and not mess around with Deref traits in the first place.

2 Likes
5

I don't understand. Please help explain

no.1

#[repr(C)]

What is this macro used for

no.2

 let mut x = RefWithOwner::new(&()).map::<Mut<U>, _>(|_, _| None);

<Mut<U>, _>(|_, _| None); I didn't understand

6

there no concept of borrowing in rust？I only know variable reference and variable immutable reference. If I understand wrong, please help correct it. Thanks

7

This isn’t a macro, it’s an attribute used to specify the in-memory layout of the enum (so that code can rely on it). Its typical use case is sending structs over FFI which has a specific requirement for the layout (because the default repr, Rust, doesn’t guarantee anything and so is unsuitable).

The ::<Mut<U>, _> just uses the Turbofish to specify the generic parameters of the function — the first is Mut<U> and the second is unspecified (to be inferred by the compiler). The actual function call is .map(|_, _| None) i.e. it passes in a closure that takes two parameters but always returns None.

1 Like
8

Yep, I remember wrong :smile:. It's an attribute. Thanks for the second answer

9

Thank you all for your feedback, just a heads up that it may be a while before I can put more time into this, and I will need more time to dissect @steffahn's example before I can figure out if it's the death knell for this library or if it's something that can be avoided with a change to the API.

Thanks @SabrinaJewson for pointing out the aliasing issue, I think I understand that one a little better, though I admit I'm still a little fuzzy on it and probably should read up on what noalias means again.

10

I should probably give more explanation for my example.

The problem it exploits is the borrowed_mut and borrowed API returning a reference to <B as BorrowWithLifetime<'a>>::Borrowed, where 'a is a user-controlled lifetime, the lifetime of the &'a self or &'a mut self receiver type.

However, the type <B as BorrowWithLifetime<'a>>::Borrowed might actually be invariant, which means you must not turn <B as BorrowWithLifetime<'a1>>::Borrowed into <B as BorrowWithLifetime<'a2>>::Borrowed for different lifetimes 'a1 and 'a2. For example the ouroboros crate approaches this general problem by

  • either only providing a .with... method that calls a callback of type -- expressed in terms of borrowed_with_owner API -- for<'b> FnOnce(&'a <B as BorrowWithLifetime<'b>>::Borrowed) -> ..., so that the argument 'b would keep being a single, unspecified lifetime that's only known to last longer than the 'a from the &'a self borrow. (I suspect such a .with_borrowed/.with_borrowed_mut method would also need an additional dummy parameter like .map in order to make the compiler happe.)

  • or in case a direct accessor is provided, the covarance of the field-type is ensured (by generating some code in the macro that fails to compiler if the type isn't covariant).

The choice is left to the user, with an annotation on the field. Allowing opt-in for covariant-only types in the case of borrowed_with_owner's API would probably require a second trait, and that one would need to be unsafe, but there could be a macro provided for safely implementing it whilst checking the covariance.

1 Like
11

Oh I see, that makes a lot of sense actually. .borrowed() and .borrowed_mut() change the lifetime in the borrowed type, which is unsound if the borrowed type is invariant with respect to that lifetime. But actually now that I think about it, does it even matter whether the borrowed type is invariant? For example, this compiles, and it only uses &-references, which are covariant:

enum RefOrPtr<'a> {
    Ref(&'a &'static str),
    Ptr(*const &'static str),
}

let mut ref_with_owner = RefWithOwner::new(&&"abc");

let mut ref_or_ptr = RefOrPtr::Ref(&"def");

if let RefOrPtr::Ref(ref_) = &ref_or_ptr {
    *ref_with_owner.borrowed_mut() = ref_;
}

ref_or_ptr = RefOrPtr::Ptr(std::ptr::null());

// Segmentation Fault, null pointer dereferenced
dbg!(ref_with_owner.borrowed());

It does sound like switching to with_borrowed and with_borrowed_mut would fix it though, which is good news. I think I actually had those methods at one point, and removed them when I realized .borrowed() and .borrowed_mut() were more powerful, but it looks like they are necessary after all.

1 Like
12

Ah, looks like I misremembered the ouroboros API, borrowed_mut is completely unsound, and something like it is also absolutely not part of the API that ouroboros' macro generates; and borrowed alone is the thing that's okay (but only okay in case of an covariant type). Note that in case of covariant type it should however also be possible to implement borrowed in terms of with_borrowed on the user-side, which is to say that only providing the latter is not limiting the user's abilities in any way, except for the fact that it's more lengthy to write something like with_borrowed(|r| r) instead of borrowed().

1 Like
13

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.