From 0224f9fa3850f2b67eb9c780e168be1eab96dbd0 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Mon, 19 Oct 2026 21:45:58 -0300 Subject: [PATCH] nvx-rs: make the guest failure reporters return their CommandError nvx-console-pending, nvx-mmio-write, and nvx-reseed report each failed system call with a reporter and then fail with CommandError::Operation. Their reporters returned nothing, so ten call sites repeated both steps in a block: the open, FIONREAD, and close sites of console_pending's fail closure, the open, mmap, munmap, and close sites of mmio_write's perror, and the open and read sites of the fail closure of reseed's read_seed. reseed() also wrote read_seed's "nvx-reseed: open PATH: REASON" diagnostic a second time, inline, for /dev/random. Each reporter now returns CommandError::Operation, as ingress::usage and managed_agent::fatal return their failures, so each site is one expression, such as .map_err(|error| perror(err, b"open /dev/mem", error))?, or return Err(file_failure(...)). In reseed, the function file_failure replaces read_seed's closure and also reports reseed()'s failure to open /dev/random, so that diagnostic has one definition. Behavior is unchanged. Every site writes the same bytes with one sys::report write, as before, and fails with the same CommandError::Operation, so the programs' standard error and exit status 1 do not change. Only the guest programs, which build for Linux with the guest feature, change. The unit tests of the three modules check these diagnostics byte for byte: console_pending's open and FIONREAD failures, mmio_write's open and mmap failures, and reseed's open and read failures of the seed file and its failure to open /dev/random, the last two through the same expected-message helper. tests/guest_tools.rs checks the open failures of nvx-console-pending and nvx-mmio-write in the built binaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nvx-rs/src/guest/console_pending.rs | 19 +++++--------- nvx-rs/src/guest/mmio_write.rs | 30 +++++++++------------- nvx-rs/src/guest/reseed.rs | 40 +++++++++++------------------ 3 files changed, 34 insertions(+), 55 deletions(-) diff --git a/nvx-rs/src/guest/console_pending.rs b/nvx-rs/src/guest/console_pending.rs index 34acde7e6..7eb84aefd 100644 --- a/nvx-rs/src/guest/console_pending.rs +++ b/nvx-rs/src/guest/console_pending.rs @@ -40,22 +40,17 @@ fn run(args: &[CString], out: &mut dyn Write, err: &mut dyn Write) -> Result<(), b"\n", ], ); + CommandError::Operation }; let flags = OpenFlags::O_RDONLY | OpenFlags::O_CLOEXEC | OpenFlags::O_NOCTTY | OpenFlags::O_NONBLOCK; - let descriptor = sys::open(device, flags, Mode::empty()).map_err(|error| { - fail(err, b"open", error); - CommandError::Operation - })?; - let pending = sys::console_pending(&descriptor).map_err(|error| { - fail(err, b"ioctl", error); - CommandError::Operation - })?; - descriptor.close().map_err(|error| { - fail(err, b"close", error); - CommandError::Operation - })?; + let descriptor = + sys::open(device, flags, Mode::empty()).map_err(|error| fail(err, b"open", error))?; + let pending = sys::console_pending(&descriptor).map_err(|error| fail(err, b"ioctl", error))?; + descriptor + .close() + .map_err(|error| fail(err, b"close", error))?; if let Err(errno) = sys::print(out, format!("{pending}\n").as_bytes()) { let text = errno.desc().as_bytes(); sys::report(err, &[b"nvx-console-pending: write: ", text, b"\n"]); diff --git a/nvx-rs/src/guest/mmio_write.rs b/nvx-rs/src/guest/mmio_write.rs index d7b69fe99..5fc47dafe 100644 --- a/nvx-rs/src/guest/mmio_write.rs +++ b/nvx-rs/src/guest/mmio_write.rs @@ -43,36 +43,30 @@ fn run(args: &[CString], memory: &CStr, err: &mut dyn Write) -> Result<(), Comma } let descriptor = sys::open(memory, OpenFlags::O_RDWR | OpenFlags::O_SYNC, Mode::empty()) - .map_err(|error| { - perror(err, b"open /dev/mem", error); - CommandError::Operation - })?; + .map_err(|error| perror(err, b"open /dev/mem", error))?; let length = NonZeroUsize::new(page_size).expect("a valid page size is nonzero"); let mut mapping = sys::MemoryMapping::shared_read_write(&descriptor, length, page_base) - .map_err(|error| { - perror(err, b"mmap /dev/mem", error); - CommandError::Operation - })?; + .map_err(|error| perror(err, b"mmap /dev/mem", error))?; if !mapping.write_u32(page_offset as usize, value) { // The boundary validation above proves this unreachable, but preserve a typed failure if // the mapping abstraction and validation ever diverge. sys::report(err, &[b"MMIO write crosses a page boundary\n"]); return Err(CommandError::Operation); } - mapping.unmap().map_err(|error| { - perror(err, b"munmap /dev/mem", error); - CommandError::Operation - })?; - descriptor.close().map_err(|error| { - perror(err, b"close /dev/mem", error); - CommandError::Operation - })?; + mapping + .unmap() + .map_err(|error| perror(err, b"munmap /dev/mem", error))?; + descriptor + .close() + .map_err(|error| perror(err, b"close /dev/mem", error))?; Ok(()) } -/// Reports `errno` as C's `perror(prefix)` does, with `nix`'s description. -fn perror(err: &mut dyn Write, prefix: &[u8], errno: Errno) { +/// Reports `errno` as C's `perror(prefix)` does, with `nix`'s description, and returns the +/// operation failure. +fn perror(err: &mut dyn Write, prefix: &[u8], errno: Errno) -> CommandError { sys::report(err, &[prefix, b": ", errno.desc().as_bytes(), b"\n"]); + CommandError::Operation } #[cfg(test)] diff --git a/nvx-rs/src/guest/reseed.rs b/nvx-rs/src/guest/reseed.rs index 1328f15d8..695367549 100644 --- a/nvx-rs/src/guest/reseed.rs +++ b/nvx-rs/src/guest/reseed.rs @@ -94,12 +94,7 @@ fn reseed(random: &CStr, seed: &[u8; SEED_BYTES], err: &mut dyn Write) -> Result OpenFlags::O_WRONLY | OpenFlags::O_CLOEXEC, Mode::empty(), ) - .map_err(|errno| { - let path = random.to_bytes(); - let text = errno.desc().as_bytes(); - sys::report(err, &[b"nvx-reseed: open ", path, b": ", text, b"\n"]); - CommandError::Operation - })?; + .map_err(|errno| file_failure(err, b"open", random, errno.desc().as_bytes()))?; sys::reseed_random(&descriptor, seed).map_err(|errno| { let text = errno.desc().as_bytes(); sys::report(err, &[b"nvx-reseed: random ioctl: ", text, b"\n"]); @@ -109,40 +104,35 @@ fn reseed(random: &CStr, seed: &[u8; SEED_BYTES], err: &mut dyn Write) -> Result /// Reads exactly `bytes.len()` bytes from the file at `path`, without retrying after `EINTR`. fn read_seed(path: &CStr, bytes: &mut [u8], err: &mut dyn Write) -> Result<(), CommandError> { - let fail = |err: &mut dyn Write, operation: &[u8], reason: &[u8]| { - let path = path.to_bytes(); - sys::report( - err, - &[b"nvx-reseed: ", operation, b" ", path, b": ", reason, b"\n"], - ); - }; let descriptor = sys::open( path, OpenFlags::O_RDONLY | OpenFlags::O_CLOEXEC, Mode::empty(), ) - .map_err(|errno| { - fail(err, b"open", errno.desc().as_bytes()); - CommandError::Operation - })?; + .map_err(|errno| file_failure(err, b"open", path, errno.desc().as_bytes()))?; let mut offset = 0; while offset < bytes.len() { let rest = &mut bytes[offset..]; match descriptor.read(rest) { - Ok(0) => { - fail(err, b"read", b"unexpected end of file"); - return Err(CommandError::Operation); - } + Ok(0) => return Err(file_failure(err, b"read", path, b"unexpected end of file")), Ok(count) => offset += count, - Err(errno) => { - fail(err, b"read", errno.desc().as_bytes()); - return Err(CommandError::Operation); - } + Err(errno) => return Err(file_failure(err, b"read", path, errno.desc().as_bytes())), } } Ok(()) } +/// Reports that `operation` on the file at `path` failed for `reason`, and returns the operation +/// failure. +fn file_failure(err: &mut dyn Write, operation: &[u8], path: &CStr, reason: &[u8]) -> CommandError { + let path = path.to_bytes(); + sys::report( + err, + &[b"nvx-reseed: ", operation, b" ", path, b": ", reason, b"\n"], + ); + CommandError::Operation +} + /// Checks that `previous` is 32 hexadecimal digits that differ from `generation_id`. fn validate_generation_change( previous: &[u8],