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}") })?); }