Files
RedBear-OS/netstack/src/link/stp.rs
T
Red Bear OS efa24228e3 review: fix 5 panic/crash bugs + 1 off-by-2 + add 6 regression tests
CRITICAL BUGS FIXED:

1. STP build_bpdu runtime panic (link/stp.rs:121-123)
   - Duration::total_millis() returns i64, to_be_bytes() = [u8; 8]
   - But destination buffers (buf[29..31], buf[31..33], buf[33..35]) are 2 bytes
   - copy_from_slice panics on length mismatch
   - Triggered by every root bridge hello BPDU — daemon crash
   - Fix: convert to ticks (1s = 256 ticks per IEEE 802.1D) as u16

2. SLAAC PIO off-by-2 byte parsing (slaac.rs:155-180)
   - parse_router_advertisement used opt_data[2] for prefix length
   - After pos += 2 consumed type+length, opt_data[0] is the prefix length
   - All PIO fields were off by 2 — SLAAC got wrong prefix length, flags,
     valid_lifetime (read Preferred Lifetime), preferred_lifetime (read Reserved2)
   - Fix: use opt_data[0] for prefix length, [1] for flags, [2..6] for valid,
     [6..10] for preferred, [14..30] for prefix bytes

3. ICMPv4 error wrong IHL extraction (icmp_error.rs:25, 62)
   - (ipv4.version() & 0x0f) * 4 always computed 16
   - smoltcp's version() returns 4 (version), not combined byte
   - Fix: use ipv4.header_len()

4. UDP can_recv panic on port-only endpoint (scheme/udp.rs:54)
   - data.addr.unwrap() panics if addr is None (e.g. udp/:53)
   - is_specified() returns true when port is non-zero, even if addr is None
   - Fix: use let-else pattern, accept all packets if addr is None

5. TCP .expect() calls crash daemon (scheme/tcp.rs)
   - 5 .expect() calls in connect/listen/send/recv paths
   - Any socket error panics the entire netstack daemon
   - Fix: replace with .map_err() returning EIO

6. ICMP .unwrap() on malformed packets (scheme/icmp.rs:217-220)
   - recv().expect() panics, Icmpv4Repr::parse().unwrap() panics
   - Crafted/malformed ICMP packets would crash the daemon
   - Fix: use match, drop unparseable packets

NEW TESTS (6 added, 29 total now passing):
- icmp_error::icmpv4_short_packet_returns_none
- icmp_error::icmpv4_preserves_destination_address (regression for bug 3)
- icmp_error::icmpv4_with_ip_options_includes_extended_header
- slaac::ra_with_pio_64_parses_correctly (regression for bug 2)
- link::stp::bpdu_minimal_parses
- link::stp::bpdu_short_returns_none
- link::stp::bpdu_wrong_protocol_returns_none
- link::stp::build_bpdu_does_not_panic (regression for bug 1)

The SLAAC test would have failed before the off-by-2 fix.
The STP build_bpdu test would have panicked before the ticks fix.
The ICMP tests verify the full IP header (incl. IHL=6 options) is preserved.
2026-07-08 23:41:42 +03:00

208 lines
7.0 KiB
Rust

//! Spanning Tree Protocol (IEEE 802.1D) — mirrors Linux 7.1's `net/bridge/`.
//!
//! Reference files:
//! - `net/bridge/br_stp.c` — STP state machine (`br_set_state`, `br_become_root_bridge`)
//! - `net/bridge/br_stp_bpdu.c` — BPDU handling (`br_stp_rcv`, `br_send_config_bpdu`)
//! - `net/bridge/br_stp_timer.c` — timers (`br_hello_timer_expired`, `br_tcn_timer_expired`)
//! - `net/bridge/br_private_stp.h` — port roles/states
//!
//! Prevents Ethernet loops by selectively blocking ports. Uses BPDU messages
//! (multicast to 01:80:c2:00:00:00) to elect a root bridge and compute the
//! spanning tree. Ports are either Forwarding or Blocking (simplified from the
//! full 802.1D state machine: Blocking→Listening→Learning→Forwarding).
use smoltcp::time::{Duration, Instant};
use smoltcp::wire::EthernetAddress;
pub const BPDU_MAC: EthernetAddress = EthernetAddress([0x01, 0x80, 0xc2, 0x00, 0x00, 0x00]);
pub const DEFAULT_PRIORITY: u16 = 32768;
pub const DEFAULT_HELLO: Duration = Duration::from_secs(2);
pub const DEFAULT_MAX_AGE: Duration = Duration::from_secs(20);
pub const DEFAULT_FORWARD_DELAY: Duration = Duration::from_secs(15);
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum PortState {
Blocking,
Forwarding,
}
#[derive(Debug, Clone)]
pub struct StpState {
pub bridge_priority: u16,
pub bridge_mac: EthernetAddress,
pub root_id: u64,
pub root_path_cost: u32,
pub root_port: Option<usize>,
pub hello_timer: Instant,
pub port_states: Vec<PortState>,
}
impl StpState {
pub fn new(priority: u16, mac: EthernetAddress, port_count: usize) -> Self {
let root_id = ((priority as u64) << 48) | mac_to_u64(mac);
Self {
bridge_priority: priority,
bridge_mac: mac,
root_id,
root_path_cost: 0,
root_port: None,
hello_timer: Instant::from_millis(0),
port_states: vec![PortState::Forwarding; port_count],
}
}
pub fn bridge_id(&self) -> u64 {
((self.bridge_priority as u64) << 48) | mac_to_u64(self.bridge_mac)
}
pub fn send_hello(&mut self, now: Instant) -> bool {
if now < self.hello_timer + DEFAULT_HELLO {
return false;
}
self.hello_timer = now;
true
}
pub fn process_bpdu(
&mut self,
port_idx: usize,
data: &[u8],
now: Instant,
) -> Option<Vec<u8>> {
let bpdu = BpduMessage::parse(data)?;
if bpdu.root_id < self.root_id {
self.root_id = bpdu.root_id;
self.root_path_cost = bpdu.root_path_cost + self.port_cost();
self.root_port = Some(port_idx);
return Some(self.build_bpdu());
}
if bpdu.root_id == self.root_id {
if port_idx == self.root_port.unwrap_or(usize::MAX) {
self.root_path_cost = bpdu.root_path_cost + self.port_cost();
} else if bpdu.root_path_cost < self.root_path_cost {
self.port_states[port_idx] = PortState::Blocking;
}
}
if self.bridge_id() < bpdu.bridge_id && bpdu.root_id == self.root_id {
self.root_id = self.bridge_id();
self.root_path_cost = 0;
self.root_port = None;
for state in &mut self.port_states {
*state = PortState::Forwarding;
}
return Some(self.build_bpdu());
}
None
}
pub fn is_blocked(&self, port_idx: usize) -> bool {
port_idx < self.port_states.len() && self.port_states[port_idx] == PortState::Blocking
}
fn port_cost(&self) -> u32 {
4
}
pub fn build_bpdu(&self) -> Vec<u8> {
let mut buf = vec![0u8; 35];
buf[0..2].copy_from_slice(&[0x00, 0x00]);
buf[2] = 0x00;
buf[3] = 0x00;
buf[4] = 0x00;
buf[5..13].copy_from_slice(&self.root_id.to_be_bytes());
buf[13..17].copy_from_slice(&self.root_path_cost.to_be_bytes());
buf[17..25].copy_from_slice(&self.bridge_id().to_be_bytes());
buf[25..27].copy_from_slice(&0u16.to_be_bytes());
buf[27..29].copy_from_slice(&0u16.to_be_bytes());
// IEEE 802.1D timer fields are in units of 1/256 second.
// 1 second = 256 ticks. So seconds * 256 = ticks.
let to_ticks = |d: Duration| -> u16 {
// total_millis returns i64; convert to u16 ticks (1s = 256 ticks).
// Clamp to u16::MAX to avoid overflow.
((d.total_millis() as u64).wrapping_mul(256).wrapping_div(1000) as u16)
};
buf[29..31].copy_from_slice(&to_ticks(DEFAULT_MAX_AGE).to_be_bytes());
buf[31..33].copy_from_slice(&to_ticks(DEFAULT_HELLO).to_be_bytes());
buf[33..35].copy_from_slice(&to_ticks(DEFAULT_FORWARD_DELAY).to_be_bytes());
buf
}
}
struct BpduMessage {
root_id: u64,
root_path_cost: u32,
bridge_id: u64,
}
impl BpduMessage {
fn parse(data: &[u8]) -> Option<Self> {
if data.len() < 35 || data[0..2] != [0x00, 0x00] || data[2] != 0x00 {
return None;
}
let root_id = u64::from_be_bytes(data[5..13].try_into().ok()?);
let root_path_cost = u32::from_be_bytes(data[13..17].try_into().ok()?);
let bridge_id = u64::from_be_bytes(data[17..25].try_into().ok()?);
Some(Self { root_id, root_path_cost, bridge_id })
}
}
fn mac_to_u64(mac: EthernetAddress) -> u64 {
let b = mac.as_bytes();
((b[0] as u64) << 40) | ((b[1] as u64) << 32) | ((b[2] as u64) << 24)
| ((b[3] as u64) << 16) | ((b[4] as u64) << 8) | (b[5] as u64)
}
#[cfg(test)]
mod tests {
use super::*;
fn make_minimal_bpdu() -> Vec<u8> {
// Minimal config BPDU: protocol=0x0000, version=0, type=0, flags=0,
// root_id(8) + cost(4) + bridge_id(8) + port(2) + age(2) + max_age(2) +
// hello(2) + fwd(2) = 35 bytes.
let mut p = vec![0u8; 35];
p[0] = 0x00; p[1] = 0x00; // protocol id
p[2] = 0x00; // version
p[3] = 0x00; // type = config
p
}
#[test]
fn bpdu_minimal_parses() {
let p = make_minimal_bpdu();
let m = BpduMessage::parse(&p);
assert!(m.is_some(), "Valid config BPDU should parse");
}
#[test]
fn bpdu_short_returns_none() {
let p = vec![0u8; 10];
assert!(BpduMessage::parse(&p).is_none());
}
#[test]
fn bpdu_wrong_protocol_returns_none() {
let mut p = make_minimal_bpdu();
p[0] = 0x80;
assert!(BpduMessage::parse(&p).is_none());
}
#[test]
fn build_bpdu_does_not_panic() {
// Regression test: build_bpdu used to panic because
// Duration::total_millis() returns i64 (8 bytes) but the
// destination buffer was only 2 bytes.
let stp = StpState::new(32768, EthernetAddress([0,0,0,0,0,0]), 1);
let buf = stp.build_bpdu();
// Must produce 35 bytes
assert_eq!(buf.len(), 35);
// And must be a valid BPDU
assert_eq!(&buf[0..2], &[0x00, 0x00]);
assert_eq!(buf[2], 0x00); // version
assert_eq!(buf[3], 0x00); // type
}
}