Implement read_buf and vectored read/write for SGX stdio by thaliaarchi · Pull Request #137355 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation25 Commits3 Checks6 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

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

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

rustbot added O-SGX

Target: SGX

S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

T-libs

Relevant to the library team, which will review and decide on the PR/issue.

labels

Feb 21, 2025

rustbot added S-blocked

Status: Blocked on something else such as an RFC or other implementation work.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

The versions of read and write in usercalls are vectored

This was explicitly requested by the T-libs reviewer.

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.

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.

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

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.

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 UserSafeCopyDestination for T { fn as_mut_ptr(&mut self) -> *mut T { self as _ } }

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

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

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

impl<T: ?Sized> UserRef { pub fn copy_to_enclave<V: ?Sized + UserSafeCopyDestination>(&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), ); } } }

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.)

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.

Jethro Beekman and others added 2 commits

March 10, 2025 00:45

Co-authored-by: Thalia Archibald thalia@archibald.dev

rustbot added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

and removed S-blocked

Status: Blocked on something else such as an RFC or other implementation work.

labels

Mar 10, 2025

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).

bors added S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

and removed S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

labels

Mar 12, 2025

📌 Commit c62aa0b has been approved by ChrisDenton

It is now in the queue for this repository.

bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Mar 12, 2025

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Mar 13, 2025

…x, r=ChrisDenton

Implement read_buf and vectored read/write for SGX stdio

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 rust-lang#136756

bors added a commit to rust-lang-ci/rust that referenced this pull request

Mar 13, 2025

…iaskrgr

Rollup of 9 pull requests

Successful merges:

• rust-lang#126856 (remove deprecated tool rls)
• rust-lang#133981 (rustdoc-json: Refractor and document Id's)
• rust-lang#136842 (Add libstd support for Trusty targets)
• rust-lang#137355 (Implement read_buf and vectored read/write for SGX stdio)
• rust-lang#137457 (fix for issue 132802: x86 code in wasm32-unknown-unknown binaries)
• rust-lang#138162 (Update the standard library to Rust 2024)
• rust-lang#138273 (metadata: Ignore sysroot when doing the manual native lib search in rustc)
• rust-lang#138346 (naked functions: on windows emit .endef without the symbol name)
• rust-lang#138370 (Simulate OOM for the try_oom_error test)

r? @ghost @rustbot modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

Mar 13, 2025

…iaskrgr

Rollup of 8 pull requests

Successful merges:

• rust-lang#126856 (remove deprecated tool rls)
• rust-lang#133981 (rustdoc-json: Refractor and document Id's)
• rust-lang#136842 (Add libstd support for Trusty targets)
• rust-lang#137355 (Implement read_buf and vectored read/write for SGX stdio)
• rust-lang#138162 (Update the standard library to Rust 2024)
• rust-lang#138273 (metadata: Ignore sysroot when doing the manual native lib search in rustc)
• rust-lang#138346 (naked functions: on windows emit .endef without the symbol name)
• rust-lang#138370 (Simulate OOM for the try_oom_error test)

r? @ghost @rustbot modify labels: rollup

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Mar 13, 2025

Rollup merge of rust-lang#137355 - thaliaarchi:io-optional-methods/sgx, r=ChrisDenton

Implement read_buf and vectored read/write for SGX stdio

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 rust-lang#136756

github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request

Mar 14, 2025

…x, r=ChrisDenton

Implement read_buf and vectored read/write for SGX stdio

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 rust-lang#136756

thaliaarchi added a commit to thaliaarchi/rust that referenced this pull request

Mar 18, 2025

In rust-lang#108326, read_buf was implemented for a variety of types, but SGX was saved for later. Update a test from then, now that rust-lang#137355 implemented it for SGX types.

Kobzol added a commit to Kobzol/rust that referenced this pull request

Mar 21, 2025

…orkingjubilee

Update test for SGX now implementing read_buf

In rust-lang#108326, read_buf was implemented for a variety of types, but SGX was saved for later. Update a test from then, now that rust-lang#137355 implemented it for SGX types.

cc @jethrogb

tgross35 added a commit to tgross35/rust that referenced this pull request

Mar 22, 2025

…orkingjubilee

Update test for SGX now implementing read_buf

In rust-lang#108326, read_buf was implemented for a variety of types, but SGX was saved for later. Update a test from then, now that rust-lang#137355 implemented it for SGX types.

cc @jethrogb

jieyouxu added a commit to jieyouxu/rust that referenced this pull request

Mar 23, 2025

…orkingjubilee

Update test for SGX now implementing read_buf

In rust-lang#108326, read_buf was implemented for a variety of types, but SGX was saved for later. Update a test from then, now that rust-lang#137355 implemented it for SGX types.

cc @jethrogb

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Mar 23, 2025

…orkingjubilee

Update test for SGX now implementing read_buf

In rust-lang#108326, read_buf was implemented for a variety of types, but SGX was saved for later. Update a test from then, now that rust-lang#137355 implemented it for SGX types.

cc @jethrogb

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Mar 23, 2025

…orkingjubilee

Update test for SGX now implementing read_buf

In rust-lang#108326, read_buf was implemented for a variety of types, but SGX was saved for later. Update a test from then, now that rust-lang#137355 implemented it for SGX types.

cc @jethrogb

compiler-errors added a commit to compiler-errors/rust that referenced this pull request

Mar 23, 2025

…orkingjubilee

Update test for SGX now implementing read_buf

In rust-lang#108326, read_buf was implemented for a variety of types, but SGX was saved for later. Update a test from then, now that rust-lang#137355 implemented it for SGX types.

cc @jethrogb

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Mar 24, 2025

Rollup merge of rust-lang#138631 - thaliaarchi:sgx-read-buf-test, r=workingjubilee

Update test for SGX now implementing read_buf

In rust-lang#108326, read_buf was implemented for a variety of types, but SGX was saved for later. Update a test from then, now that rust-lang#137355 implemented it for SGX types.

cc @jethrogb

github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request

Apr 2, 2025

In rust-lang#108326, read_buf was implemented for a variety of types, but SGX was saved for later. Update a test from then, now that rust-lang#137355 implemented it for SGX types.

github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request

Apr 2, 2025

…orkingjubilee

Update test for SGX now implementing read_buf

In rust-lang#108326, read_buf was implemented for a variety of types, but SGX was saved for later. Update a test from then, now that rust-lang#137355 implemented it for SGX types.

cc @jethrogb

Labels

O-SGX

Target: SGX

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

T-libs

Relevant to the library team, which will review and decide on the PR/issue.