Skip to content

Idea for disabling interior mutability that plays nicely with Stacked Borrows #1760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
joshlf opened this issue Sep 26, 2024 · 5 comments
Open

Comments

@joshlf
Copy link
Member

joshlf commented Sep 26, 2024

See it on the Rust Playground

use core::cell::UnsafeCell;
use zerocopy::AsBytes;

// INVARIANT: A pointer to `Self` is never created with `SharedReadWrite`
// permissions.
#[repr(transparent)]
pub struct ReadOnly<T>(T);

impl<T> ReadOnly<T> {
    fn new(t: T) -> ReadOnly<T> {
        ReadOnly(t)
    }
}

impl<T> ReadOnly<T> {
    fn from_mut<'a>(t: &'a mut T) -> &'a mut ReadOnly<T> {
        let t_ptr: *mut T = t;
        // INVARIANT: Because `t: &mut T`, `t_ptr` has `Unique` permissions, not
        // `SharedReadWrite` permissions. This is true even if `T` contains
        // `UnsafeCell`s.
        unsafe { &mut *t_ptr.cast::<ReadOnly<T>>() }
    }
}

impl<T: AsBytes> ReadOnly<UnsafeCell<T>> {
    fn as_bytes(&self) -> &[u8] {
        let len = size_of_val(self);
        let slf: *const Self = self;
        // SAFETY: Since, by invariant, a pointer to `Self` is never constructed
        // with `SharedReadWrite` permissions, we know that `self` must only
        // have `Shared` permissions. Thus, this operation does not invalidly
        // re-tag the pointer's permissions.
        //
        // Since `T: AsBytes` and since `UnsafeCell<T>` has the same bit
        // validity as `T`, none of the bytes of `*self` are uninitialized.
        unsafe { core::slice::from_raw_parts(slf.cast::<u8>(), len) }
    }
}

fn main() {
    // By value
    let ro = ReadOnly::new(UnsafeCell::new(0u8));
    println!("{:?}", ro.as_bytes());

    // By reference
    let mut val = UnsafeCell::new(0u8);
    let ro = ReadOnly::from_mut(&mut val);
    println!("{:?}", ro.as_bytes());
}

The key idea is that, while Stacked Borrows can't be used to disable UnsafeCells if you've started out with a &UnsafeCell, which has SharedReadWrite permissions (see rust-lang/unsafe-code-guidelines#303), it's still sound so long as you never create a pointer with SharedReadWrite permissions in the first place. You can do that by only constructing a ReadOnly by value or by &mut.

@joshlf
Copy link
Member Author

joshlf commented Sep 26, 2024

@RalfJung Does this look sound to you?

@RalfJung
Copy link

RalfJung commented Sep 29, 2024

I am confused. When &mut ReadOnly<UnsafeCell<T>> gets turned into &ReadOnly<UnsafeCell<T>>, that will result in SharedReadWrite permission. You can't really prevent that. So how is the invariant supposed to be upheld?

I don't think I understand what you are trying to achieve, and the comments don't explain that either.

t_ptr has Write permissions

Stacked Borrows has no Write permission. If this is supposed to be SB terminology, you probably mean Unique.

@joshlf
Copy link
Member Author

joshlf commented Sep 29, 2024

I am confused. When &mut ReadOnly<UnsafeCell<T>> gets turned into &ReadOnly<UnsafeCell<T>>, that will result in SharedReadWrite permission. You can't really prevent that. So how is the invariant supposed to be upheld?

I don't think I understand what you are trying to achieve, and the comments don't explain that either.

My goal is to be able to convert a &T to a &[u8] (specifically, to implement AsBytes, which permits &T -> &[u8] via as_bytes) for types which contain UnsafeCells. IIUC, that is unsound in the general case (ie, if we don't somehow prevent the UnsafeCells from being used to perform interior mutation). rust-lang/unsafe-code-guidelines#303 implies that at least one way of "disabling" UnsafeCells doesn't play nicely with Stacked Borrows.

My hope was that, with this example, I'd found a way to disable UnsafeCells that does play nicely with Stacked Borrows. IIUC, the issue with rust-lang/unsafe-code-guidelines#303 is that going from &T -> &ReadOnly<T> generates a reference with SharedReadWrite permissions (in that example, ReadOnly contains an UnsafeCell<T>). My hope with my example is to make it so that you can only produce a new &ReadOnly<UnsafeCell<T>> by going by way of &mut UnsafeCell<T> -> &mut ReadOnly<UnsafeCell<T>>, which doesn't produce a SharedReadWrite reference.

I think I understand your point, which is that &mut ReadOnly<UnsafeCell<T>> -> &ReadOnly<UnsafeCell<T>> nonetheless produces a SharedReadWrite reference. However, even given that insight, I haven't been able to use that API to produce UB (according to Miri-with-SB). Do you believe that the API in this example is unsound? (Specifically, the combination of the new, from_mut, and as_bytes methods) Or is it the case that, despite your observation about being able to produce SharedReadWrite references, it's still sound?

t_ptr has Write permissions

Stacked Borrows has no Write permission. If this is supposed to be SB terminology, you probably mean Unique.

It is, and I do 🙂

@RalfJung
Copy link

My goal is to be able to convert a &T to a &[u8]

What does this even have to do with rust-lang/unsafe-code-guidelines#303? That issue is about converting e.g. &[u8] to &[Cell<u8>]. The opposite direction is fine under Stacked Borrows without any tricks. The standard library even relies on this, just consider RefCell<u8> on which you call borrow().

@joshlf
Copy link
Member Author

joshlf commented Sep 29, 2024

Ah okay that's good to know! I had misinterpreted rust-lang/unsafe-code-guidelines#303 as implying that SB couldn't reason about "disabling interior mutability" in both directions (ie, &T -> &UnsafeCell<T> and vice-versa).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants