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<usize> 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.
This commit is contained in:
+12
-18
@@ -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
|
||||
}
|
||||
};
|
||||
|
||||
@@ -34,7 +34,16 @@ pub fn reader_pool_from_fds(
|
||||
) -> Result<ReaderPool<OwnedFd>, 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}")
|
||||
})?);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user