From 2dba10b7df6b44f5f0064f270fb4fe20ba36058f Mon Sep 17 00:00:00 2001 From: Red Bear OS Date: Mon, 27 Jul 2026 20:13:36 +0900 Subject: [PATCH] netstack: fix RawFd call-site and complete BufferPool zero-fill Two review-identified blocking fixes: 1. RawFd compile error in corec12_integration.rs:45-55. reader_pool_from_fds received Vec from callers but the OwnedFd::from_raw_fd signature takes std::os::fd::RawFd (c_int). Per Rust's no-implicit-conversion rule this would not compile. Fix: explicit try_from(raw) with overflow error message. Add a precise SAFETY comment documenting that 'raw' is expected from libredox::Fd::into_raw_fd on a live, owned descriptor and that dup creates an independently owned duplicate. 2. BufferPool still exposed stale bytes in F001 fix: v.fill(0) only clears 0..v.len() (the OLD logical length), then unsafe { v.set_len(capacity) } extends the Vec to full capacity, leaving the tail bytes uninitialized AND violating Vec::set_len's contract (the new elements must already be initialized). This was information disclosure (prior flow data in the uninitialized tail) AND undefined behavior. Fix: use the safe v.resize(capacity, 0), which guarantees every byte in [0, capacity) is initialized to 0 before the length changes. No unsafe needed. --- netstack/src/buffer_pool.rs | 30 ++++++++++++----------------- netstack/src/corec12_integration.rs | 11 ++++++++++- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/netstack/src/buffer_pool.rs b/netstack/src/buffer_pool.rs index f06d98fc53..85de46e41d 100644 --- a/netstack/src/buffer_pool.rs +++ b/netstack/src/buffer_pool.rs @@ -81,25 +81,19 @@ impl BufferPool { Some(mut v) => { // The recycled Vec still holds the previous packet's bytes // between its logical length and its capacity. The previous - // implementation called `set_len(capacity)` and handed the - // buffer out without zeroing, which is a persistent - // information-disclosure vector: any consumer that fails to - // fully overwrite the buffer (padding bytes, partial reads) - // would leak prior flows' data on the wire. + // fix used `v.fill(0)` then `unsafe { v.set_len(capacity) }`, + // but `fill(0)` only clears the OLD logical length + // (0..v.len()), then `set_len(capacity)` extends the Vec to + // its full capacity — leaving the tail bytes uninitialized. + // That is information disclosure (prior flow data leaked + // through padding/partial reads) AND undefined behavior per + // `Vec::set_len`'s contract (the new elements must already be + // initialized). // - // Zero the capacity-then-truncate window. The cost is one - // memset of the buffer's capacity; this is acceptable because - // (a) the buffer is hot in cache, (b) the alternative is a - // security boundary violation, and (c) callers may still use - // the buffer with `resize`/`set_len` semantics as before. - let capacity = v.capacity(); - v.fill(0); - // SAFETY: capacity bytes have just been initialized to 0, - // and the previous allocation lives until the next `Drop` of - // this Vec (the Vec is not dropped, just truncated). The - // returned Vec has length == capacity and all bytes == 0, - // so reads of any prefix are well-defined. - unsafe { v.set_len(capacity) }; + // The safe, complete fix: use `v.resize(capacity, 0)`, which + // guarantees every byte in [0, capacity) is initialized to + // the new value `0` before the length changes. No `unsafe`. + v.resize(v.capacity(), 0); v } }; diff --git a/netstack/src/corec12_integration.rs b/netstack/src/corec12_integration.rs index 8e0ccdbd04..ee0e349333 100644 --- a/netstack/src/corec12_integration.rs +++ b/netstack/src/corec12_integration.rs @@ -34,7 +34,16 @@ pub fn reader_pool_from_fds( ) -> Result, String> { let mut inputs = Vec::with_capacity(raw_fds.len()); for raw in raw_fds { - inputs.push(OwnedFd::from_raw_fd(raw).map_err(|e| { + // SAFETY: `raw` came from `libredox::Fd::into_raw_fd()` on a + // live, owned file descriptor. The `RawFd` (c_int) round-trip + // succeeds for every valid fd. `dup` validates the descriptor + // through the kernel and creates an independently owned + // duplicate; the caller need not hold exclusive ownership + // of the original. + let raw_fd = std::os::fd::RawFd::try_from(raw).map_err(|e| { + format!("netstack: raw fd value {raw} does not fit in RawFd: {e}") + })?; + inputs.push(OwnedFd::from_raw_fd(raw_fd).map_err(|e| { format!("netstack: failed to dup raw fd {raw}: {e}") })?); }