Implement read_buf and vectored read/write for SGX stdio

[thaliaarchi]

Implement read_buf, read_vectored, and write_vectored for the SGX stdio types.

Additionally, extend User<T>::copy_to_enclave to work for copying to uninitialized values and fix unsoundness in UserRef<[T]>::copy_to_enclave_vec.

cc @jethrogb

Tracked in #136756

[rustbot]

r? @ChrisDenton

rustbot has assigned @ChrisDenton.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[thaliaarchi]

@rustbot blocked (on #136780)

[jethrogb]

cc @raoulstrackx

[jethrogb]

Needing this “feels” wrong to me. You could do impl<T: UserSafeSized> UserSafeSized for MaybeUninit<T> and then just create a [MaybeUninit<u8>] in read_buf. However, of course the whole point of UserSafe is that you want to explicitly assume everything you copy out of userspace is initialized, so that would kind of defeat the point.

[thaliaarchi]

If both of those options are unsatisfying to you, what would you recommend I do so this can impl read_buf?

[jethrogb]

The versions of read and write in usercalls are vectored

This was explicitly requested by the T-libs reviewer.

[thaliaarchi]

The versions of read and write in usercalls are vectored

This was explicitly requested by the T-libs reviewer.

I'm fine with reverting my change to this. Since new_from_enclave just defers to new_uninit_bytes and copy_from_enclave, which is what the vectored version uses, the only difference is saving a few arithmetic ops.

[thaliaarchi]

I noticed that UserRef<[T]>::copy_to_enclave_vec reinterprets uninitialized memory as initialized and does not drop existing elements of the Vec, so I pushed a fix for it which uses the new copy_to_enclave_uninit. I think this supports the need for such a routine.

[jethrogb]

Perhaps copy_to_enclave needs to be replaced in its entirety with your version?

[thaliaarchi]

Perhaps copy_to_enclave needs to be replaced in its entirety with your version?

I think the two would actually be better coexisting, since they serve slightly different purposes. copy_to_enclave copies from a UserRef<T> to a &mut T, whereas copy_to_enclave_uninit copies from a UserRef<[T]> to a &mut [MaybeUninit<T>]. Since MaybeUninit<T> requires T: Sized, a routine which copies from a UserRef<T> to a &mut MaybeUninit<T> wouldn't work for slices.

[jethrogb]

How about this?

// SAFETY: Requires that `T` is contained within `Self` using transparent representation
unsafe trait UserSafeCopyDestination<T: ?Sized> {
    fn as_mut_ptr(&mut self) -> *mut T;
}

unsafe impl<T> UserSafeCopyDestination<T> for T {
    fn as_mut_ptr(&mut self) -> *mut T {
        self as _
    }
}

unsafe impl<T> UserSafeCopyDestination<[T]> for [T] {
    fn as_mut_ptr(&mut self) -> *mut [T] {
        self as _
    }
}

unsafe impl<T> UserSafeCopyDestination<T> for MaybeUninit<T> {
    fn as_mut_ptr(&mut self) -> *mut T {
        self as *mut Self as _
    }
}

unsafe impl<T> UserSafeCopyDestination<[T]> for [MaybeUninit<T>] {
    fn as_mut_ptr(&mut self) -> *mut [T] {
        self as *mut Self as _
    }
}

impl<T: ?Sized> UserRef<T> {
    pub fn copy_to_enclave<V: ?Sized + UserSafeCopyDestination<T>>(&self, dest: &mut V) {
        unsafe {
            assert_eq!(mem::size_of_val(dest), mem::size_of_val(&*self.0.get()));
            copy_from_userspace(
                self.0.get() as *const T as *const u8,
                dest.as_mut_ptr() as *mut u8,
                mem::size_of_val(dest),
            );
        }
    }
}

[thaliaarchi]

That's much more flexible! I've added a commit with your patch, which I attributed to you. You might want to double-check that the metadata looks good.

(FYI, it looks like your fortanix.com email isn't connected to your GitHub account, so you're not linked.)

[thaliaarchi]

And since we've been talking about SGX copying APIs, I think UserRef<[T]>::copy_to_enclave_vec would be slightly better if it didn't clear existing elements and instead just appended. It's easy for the caller to .clear(), if they want, and this seems to better match other APIs that take a &mut Vec<T>.

That would be a breaking change, but I see no use of it outside of std.

[bors]

☔ The latest upstream changes (presumably #138155) made this pull request unmergeable. Please resolve the merge conflicts.

[thaliaarchi]

Now that #136780 has merged, we're good to go!

@rustbot review

[jethrogb]

And since we've been talking about SGX copying APIs, I think UserRef<[T]>::copy_to_enclave_vec would be slightly better if it didn't clear existing elements and instead just appended. It's easy for the caller to .clear(), if they want, and this seems to better match other APIs that take a &mut Vec.

Good idea. But then let's also change the name to better describe the behavior.

[thaliaarchi]

I've now done so and renamed it to append_to_enclave_vec.

[ChrisDenton]

r=me if @jethrogb is happy with this PR.

[jethrogb]

@ChrisDenton yes please approve

[ChrisDenton]

Thanks!

@bors r+

[bors]

📌 Commit e80df9c has been approved by ChrisDenton

It is now in the queue for this repository.

[thaliaarchi]

I had neglected to update the set_len for the append-style logic. I've pushed a fix for that (so it needs a reapproval).