review: 4 critical fixes (ICMP queue, refcount, udp panic, vlan send)
CRITICAL BUGS FIXED: 1. router/mod.rs: ICMP errors for Unreachable/Prohibit went to rx_buffer (infinite loop back to input) instead of tx_buffer (sent to source). Combined Unreachable/Prohibit arms; both now use tx_buffer. 2. scheme/socket.rs dup(): refcount leak in update_with branch. OLD code: new_handle.socket_handle() got +2 when update_with was Some, but only decremented once on close. Net: SH1 over-counted (+1), SH2 (new listening socket from update_with) never tracked at all. FIX: in update_with branch, increment refcount of 'socket_handle' (the new SH), not new_handle.socket_handle(). The always-run increment at the bottom covers new_handle. Both increments serve different purposes and are now distinct. 3. scheme/udp.rs: 4 .expect() panic vectors in bind/send/recv. 'Can't bind', 'Can't send', 'Can't receive', 'Can't recieve' all panicked the daemon. Now return EIO via ? operator. 4. link/vlan.rs (and vxlan/gre/ipip already partially fixed in R42): send() pushed tagged packets into self.recv_queue, creating a self-loop where packets were never delivered. Now drops packets with a debug log since no parent device reference exists. 5. link/qdisc.rs: TokenBucket token_add could overflow u64 on long elapsed durations. Changed to saturating_mul. DOC FIX: 6. filter/table.rs docstring example used --sport 1024:65535 (port range) but parse_port only accepts single port. Changed example to use single port value. Range support is a future enhancement. All 29 existing tests still pass.
This commit is contained in:
@@ -17,7 +17,7 @@
|
||||
//! Examples:
|
||||
//! - `ACCEPT input -p tcp --dport 80`
|
||||
//! - `DROP input -s 10.0.0.0/8`
|
||||
//! - `ACCEPT output -d 192.168.1.0/24 --sport 1024:65535`
|
||||
//! - `ACCEPT output -d 192.168.1.0/24 --sport 1024`
|
||||
|
||||
extern crate alloc;
|
||||
|
||||
|
||||
@@ -146,10 +146,9 @@ impl GreDevice {
|
||||
}
|
||||
|
||||
impl LinkDevice for GreDevice {
|
||||
fn send(&mut self, _next_hop: IpAddress, packet: &[u8], now: Instant) {
|
||||
let encapsulated = self.build_encapsulated(packet).to_vec();
|
||||
self.push_received(encapsulated);
|
||||
let _ = now;
|
||||
fn send(&mut self, _next_hop: IpAddress, packet: &[u8], _now: Instant) {
|
||||
// No parent device reference — drop rather than self-loop.
|
||||
log::debug!("gre: dropping {} byte frame (no parent device)", packet.len());
|
||||
}
|
||||
|
||||
fn recv(&mut self, _now: Instant) -> Option<&[u8]> {
|
||||
|
||||
@@ -80,10 +80,9 @@ impl IpipDevice {
|
||||
}
|
||||
|
||||
impl LinkDevice for IpipDevice {
|
||||
fn send(&mut self, _next_hop: IpAddress, packet: &[u8], now: Instant) {
|
||||
let encapsulated = self.build_outer_header(packet).to_vec();
|
||||
self.push_received(encapsulated);
|
||||
let _ = now;
|
||||
fn send(&mut self, _next_hop: IpAddress, packet: &[u8], _now: Instant) {
|
||||
// No parent device reference — drop rather than self-loop.
|
||||
log::debug!("ipip: dropping {} byte frame (no parent device)", packet.len());
|
||||
}
|
||||
|
||||
fn recv(&mut self, _now: Instant) -> Option<&[u8]> {
|
||||
|
||||
@@ -45,7 +45,11 @@ impl TokenBucket {
|
||||
pub fn consume(&mut self, bytes: u64, now: Instant) -> bool {
|
||||
let elapsed = now - self.last_update;
|
||||
if elapsed > Duration::ZERO {
|
||||
let token_add = (self.rate * elapsed.total_millis() as u64) / 8000;
|
||||
// Saturating mul: rate (bytes/s) * elapsed (ms) / 8000.
|
||||
// saturating_mul prevents overflow on long elapsed durations.
|
||||
let token_add = self.rate
|
||||
.saturating_mul(elapsed.total_millis() as u64)
|
||||
/ 8000;
|
||||
self.tokens = (self.tokens + token_add).min(self.burst);
|
||||
self.last_update = now;
|
||||
}
|
||||
|
||||
@@ -117,10 +117,13 @@ impl VlanDevice {
|
||||
}
|
||||
|
||||
impl LinkDevice for VlanDevice {
|
||||
fn send(&mut self, next_hop: IpAddress, packet: &[u8], now: Instant) {
|
||||
let tagged = self.insert_tag(packet).to_vec();
|
||||
self.push_received(tagged);
|
||||
let _ = (next_hop, now);
|
||||
fn send(&mut self, next_hop: IpAddress, packet: &[u8], _now: Instant) {
|
||||
// The VLAN device has no parent device reference, so we cannot
|
||||
// send the tagged frame out. Log and drop the packet rather than
|
||||
// creating a self-loop (which would re-deliver the packet to
|
||||
// recv() and never reach a real interface).
|
||||
log::debug!("vlan: dropping {} byte frame (no parent device)", packet.len());
|
||||
let _ = next_hop;
|
||||
}
|
||||
|
||||
fn recv(&mut self, _now: Instant) -> Option<&[u8]> {
|
||||
|
||||
@@ -176,10 +176,9 @@ impl VxlanDevice {
|
||||
}
|
||||
|
||||
impl LinkDevice for VxlanDevice {
|
||||
fn send(&mut self, _next_hop: IpAddress, packet: &[u8], now: Instant) {
|
||||
let enc = self.build_encapsulated(packet).to_vec();
|
||||
self.push_received(enc);
|
||||
let _ = now;
|
||||
fn send(&mut self, _next_hop: IpAddress, packet: &[u8], _now: Instant) {
|
||||
// No parent device reference — drop rather than self-loop.
|
||||
log::debug!("vxlan: dropping {} byte frame (no parent device)", packet.len());
|
||||
}
|
||||
|
||||
fn recv(&mut self, _now: Instant) -> Option<&[u8]> {
|
||||
|
||||
@@ -135,16 +135,13 @@ impl Router {
|
||||
|
||||
match route_type {
|
||||
RouteType::Blackhole => continue,
|
||||
RouteType::Unreachable => {
|
||||
RouteType::Unreachable | RouteType::Prohibit => {
|
||||
// Build the ICMPv4 error and queue it for transmit
|
||||
// back to the original sender. Using rx_buffer here
|
||||
// would re-route the error back into the input path
|
||||
// (infinite loop) and never reach the sender.
|
||||
if let Some(error_pkt) = icmp_error::build_icmpv4_port_unreachable(packet) {
|
||||
let _ = self.rx_buffer.enqueue(error_pkt.len(), ())
|
||||
.map(|b| b.copy_from_slice(&error_pkt));
|
||||
}
|
||||
continue;
|
||||
}
|
||||
RouteType::Prohibit => {
|
||||
if let Some(error_pkt) = icmp_error::build_icmpv4_port_unreachable(packet) {
|
||||
let _ = self.rx_buffer.enqueue(error_pkt.len(), ())
|
||||
let _ = self.tx_buffer.enqueue(error_pkt.len(), ())
|
||||
.map(|b| b.copy_from_slice(&error_pkt));
|
||||
}
|
||||
continue;
|
||||
|
||||
@@ -834,22 +834,27 @@ where
|
||||
|
||||
if let Some((socket_handle, data)) = update_with {
|
||||
if let SchemeFile::Socket(ref mut file) = *file {
|
||||
// We replace the socket_handle pointed by file so update the ref_counts
|
||||
// accordingly
|
||||
// We replace the socket_handle pointed by file so update
|
||||
// the ref_counts accordingly.
|
||||
self.ref_counts
|
||||
.entry(file.socket_handle)
|
||||
.and_modify(|e| *e = e.saturating_sub(1))
|
||||
.or_insert(0);
|
||||
|
||||
// Increment refcount of the NEW socket (socket_handle)
|
||||
// that the file is being updated to point at.
|
||||
*self
|
||||
.ref_counts
|
||||
.entry(new_handle.socket_handle())
|
||||
.entry(socket_handle)
|
||||
.or_insert(0) += 1;
|
||||
|
||||
file.socket_handle = socket_handle;
|
||||
file.data = data;
|
||||
}
|
||||
}
|
||||
// Increment refcount for the new_handle fd being inserted.
|
||||
// This is always done (regardless of update_with) because
|
||||
// new_handle represents a new fd in the handle table.
|
||||
*self
|
||||
.ref_counts
|
||||
.entry(new_handle.socket_handle())
|
||||
|
||||
@@ -178,7 +178,7 @@ impl<'a> SchemeSocket for UdpSocket<'a> {
|
||||
|
||||
udp_socket
|
||||
.bind(local_endpoint)
|
||||
.expect("Can't bind udp socket to local endpoint");
|
||||
.map_err(|_| SyscallError::new(syscall::EIO))?;
|
||||
|
||||
Ok((socket_handle, remote_endpoint))
|
||||
}
|
||||
@@ -213,7 +213,7 @@ impl<'a> SchemeSocket for UdpSocket<'a> {
|
||||
.expect("If we can send, this should be specified"),
|
||||
endpoint.port,
|
||||
);
|
||||
self.send_slice(buf, endpoint).expect("Can't send slice");
|
||||
self.send_slice(buf, endpoint).map_err(|_| SyscallError::new(syscall::EIO))?;
|
||||
Ok(buf.len())
|
||||
} else if file.flags & syscall::O_NONBLOCK == syscall::O_NONBLOCK {
|
||||
Err(SyscallError::new(syscall::EAGAIN))
|
||||
@@ -230,7 +230,7 @@ impl<'a> SchemeSocket for UdpSocket<'a> {
|
||||
if !file.read_enabled {
|
||||
Ok(0)
|
||||
} else if self.can_recv(&file.data) {
|
||||
let (length, _) = self.recv_slice(buf).expect("Can't receive slice");
|
||||
let (length, _) = self.recv_slice(buf).map_err(|_| SyscallError::new(syscall::EIO))?;
|
||||
Ok(length)
|
||||
} else if file.flags & syscall::O_NONBLOCK == syscall::O_NONBLOCK {
|
||||
Err(SyscallError::new(syscall::EAGAIN))
|
||||
@@ -358,7 +358,7 @@ impl<'a> SchemeSocket for UdpSocket<'a> {
|
||||
let mut payload_tmp = vec![0u8; prepared_whole_iov_size];
|
||||
let (length, address) = self
|
||||
.recv_slice(&mut payload_tmp)
|
||||
.expect("Can't recieve slice");
|
||||
.map_err(|_| SyscallError::new(syscall::EIO))?;
|
||||
|
||||
//Address Handling
|
||||
let address_formatted = if prepared_name_len > 0 {
|
||||
|
||||
Reference in New Issue
Block a user