Possible unsoundness with uninit to_guest: &mut [u32] for syscalls #2853
Description
I think it may be unsound for Syscall to send data from the host to guest via to_guest: &mut [u32], when the pointer given by the guest is to an uninitialized allocation. I think use of sys_read or sys_read_words with uninitialized buffers could trigger UB. I think it should instead be &mut [MaybeUninit<u32>].
My motivation for this comes from wanting to implement Read::read_buf for the RISC Zero version of std::io::Stdin (tracked in rust-lang/rust#136756).
If this sounds good, I could submit a PR here similar to what I did for the Hermit unikernel in hermit-os/kernel#1606 (Use uninitialized buffers for read and recvfrom). However, since each file descriptor is associated with a dyn Read and Read::read_buf is currently unstable, this might not be possible.
On the other hand, Rust tentatively does not have full recursive validity for references, so as long as uninitialized data is not read, you might be fine. @RalfJung?
Steps to reproduce
This patch to std, which implements Read::read_buf for RISC Zero stdin, would encounter this issue.
Since I know little about RISC Zero, here's my understanding of the relevant bits, so you can correct me if I'm wrong:
Trace from the C FFI to the underlying Read::read calls
-
Guest code invokes a function in the C FFI, passing a buffer via a
*mut u8andusizelength:risc0/risc0/zkvm/platform/src/syscall.rs
Line 482 in 711b163
-
It invokes a syscall via
ecall, passing the pointer and length bya0anda1registers and :risc0/risc0/zkvm/platform/src/syscall.rs
Lines 584 to 590 in 711b163
risc0/risc0/zkvm/platform/src/syscall.rs
Lines 236 to 251 in 711b163
-
It is somehow transmitted to the host. I'm fuzzy here. I assume the guest and host are on the same system?
-
The host resolves the syscall
risc0/risc0/zkvm/src/host/server/exec/syscall/mod.rs
Lines 197 to 199 in 711b163
from the table:
-
And calls the appropriate
impl Syscall:risc0/risc0/zkvm/src/host/server/exec/syscall/posix_io.rs
Lines 26 to 32 in 711b163
which casts
to_guestto a&mut [u8], adjusting the length to be in bytes: -
It then resolves
std::io::Readinstance for the given fd, which is either astd::io::Cursor<Vec<u8>>for stdinor some other
dyn Read:risc0/risc0/zkvm/src/host/client/posix_io.rs
Lines 62 to 64 in 711b163
-
It then calls
std::io::Read::readuntil enough of the buffer is filled:risc0/risc0/zkvm/src/host/server/exec/syscall/posix_io.rs
Lines 50 to 61 in 711b163
This API expects the buffer to be fully initialized.