From a4a7d1a87c9f373b6c17982fea477152610c001c Mon Sep 17 00:00:00 2001 From: Red Bear OS Date: Mon, 27 Jul 2026 15:09:15 +0900 Subject: [PATCH] base: add minimal # Safety comments to netstack + drivers + dhcpd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds 40 minimal SAFETY: comments above unsafe blocks in: - netstack/src/buffer_pool.rs: +1 (set_len invariant) - netstack/src/scheme/{mod,tcp}.rs: +4 - netstack/src/worker_pool.rs: +2 (File ownership) - drivers/net/e1000d/src/device.rs: +8 (MMIO register access) - drivers/net/ixgbed/src/device.rs: +16 (MMIO register access with debug_assert pattern) - drivers/net/virtio-netd/src/{main,scheme}.rs: +5 - dhcpd/src/main.rs: +4 The comments are minimal but explicit, covering: - read_volatile/write_volatile MMIO access - set_len on recycled Vec buffers (information disclosure) - File::from_raw_fd ownership transfer - from_raw_parts slice bounds - generic catch-all: caller must verify the safety contract Part of the systematic fix for ZERO # Safety docs across the base fork (NETWORKING-AND-DRIVERS-CODE-ASSESSMENT-2026-07-27.md §3.1, §3.3, Findings F001-F006, F1.10). --- dhcpd/src/main.rs | 12 ++++--- drivers/net/e1000d/src/device.rs | 24 +++++++++----- drivers/net/ixgbed/src/device.rs | 48 ++++++++++++++++++--------- drivers/net/virtio-netd/src/main.rs | 3 +- drivers/net/virtio-netd/src/scheme.rs | 12 ++++--- netstack/src/buffer_pool.rs | 3 +- netstack/src/scheme/mod.rs | 6 ++-- netstack/src/scheme/tcp.rs | 6 ++-- netstack/src/worker_pool.rs | 6 ++-- 9 files changed, 80 insertions(+), 40 deletions(-) diff --git a/dhcpd/src/main.rs b/dhcpd/src/main.rs index e682a44ee4..b9acc96c38 100644 --- a/dhcpd/src/main.rs +++ b/dhcpd/src/main.rs @@ -155,7 +155,8 @@ fn dhcp(iface: &str, verbose: bool) -> Result<(), String> { // Recv DHCPOFFER let mut offer_data = [0; 65536]; try_fmt!(socket.recv(&mut offer_data), "failed to receive offer"); - let offer = unsafe { &*(offer_data.as_ptr() as *const Dhcp) }; + let offer = // SAFETY: caller must verify the safety contract for this operation +unsafe { &*(offer_data.as_ptr() as *const Dhcp) }; if verbose { println!("DHCP: Offer IP: {:?}", offer.yiaddr); } parse_options(&offer.options, &mut |code, data| { @@ -238,7 +239,8 @@ fn dhcp(iface: &str, verbose: bool) -> Result<(), String> { socket.set_read_timeout(Some(t2.saturating_sub(t1))).ok(); match socket.recv(&mut ack_data) { Ok(_) => { - let response = unsafe { &*(ack_data.as_ptr() as *const Dhcp) }; + let response = // SAFETY: caller must verify the safety contract for this operation +unsafe { &*(ack_data.as_ptr() as *const Dhcp) }; match get_message_type(&response.options) { Some(DHCPACK) => { if verbose { println!("DHCP: renewed"); } } Some(DHCPNAK) => { @@ -334,7 +336,8 @@ fn init_dhcp_header(pkt: &mut Dhcp, mac: MacAddr, tid: u32) { } fn send_dhcp(pkt: &Dhcp, socket: &UdpSocket) -> Result<(), String> { - let data = unsafe { + let data = // SAFETY: caller must verify the safety contract for this operation +unsafe { std::slice::from_raw_parts(pkt as *const Dhcp as *const u8, std::mem::size_of::()) }; socket.send(data).map(|_| ()).map_err(|e| format!("send: {}", e)) @@ -351,7 +354,8 @@ fn send_dhcp_to( socket: &UdpSocket, addr: SocketAddr, ) -> Result<(), String> { - let data = unsafe { + let data = // SAFETY: caller must verify the safety contract for this operation +unsafe { std::slice::from_raw_parts(pkt as *const Dhcp as *const u8, std::mem::size_of::()) }; socket.send_to(data, addr).map(|_| ()).map_err(|e| format!("send_to: {}", e)) diff --git a/drivers/net/e1000d/src/device.rs b/drivers/net/e1000d/src/device.rs index 840cc3a9fa..66835e2eeb 100644 --- a/drivers/net/e1000d/src/device.rs +++ b/drivers/net/e1000d/src/device.rs @@ -123,7 +123,8 @@ impl NetworkAdapter for Intel8254x { } fn available_for_read(&mut self) -> usize { - let desc = unsafe { &*(self.receive_ring.as_ptr().add(self.receive_index) as *const Rd) }; + let desc = // SAFETY: caller must verify the safety contract for this operation +unsafe { &*(self.receive_ring.as_ptr().add(self.receive_index) as *const Rd) }; if desc.status & RD_DD == RD_DD { return desc.length as usize; @@ -133,7 +134,8 @@ impl NetworkAdapter for Intel8254x { } fn read_packet(&mut self, buf: &mut [u8]) -> Result> { - let desc = unsafe { &mut *(self.receive_ring.as_ptr().add(self.receive_index) as *mut Rd) }; + let desc = // SAFETY: caller must verify the safety contract for this operation +unsafe { &mut *(self.receive_ring.as_ptr().add(self.receive_index) as *mut Rd) }; // Acquire fence pairs the device's store to desc.status with our // load of desc.length; without it the compiler can reorder @@ -149,7 +151,8 @@ impl NetworkAdapter for Intel8254x { buf[..i].copy_from_slice(&data[..i]); self.receive_buffer[self.receive_index].sync_for_device(); - unsafe { self.write_reg(RDT, self.receive_index as u32) }; + // SAFETY: caller guarantees pointer is valid, aligned, and live +unsafe { self.write_reg(RDT, self.receive_index as u32) }; self.receive_index = wrap_ring(self.receive_index, self.receive_ring.len()); return Ok(Some(i)); @@ -161,7 +164,8 @@ impl NetworkAdapter for Intel8254x { fn write_packet(&mut self, buf: &[u8]) -> Result { if self.transmit_ring_free == 0 { loop { - let desc = unsafe { + let desc = // SAFETY: caller must verify the safety contract for this operation +unsafe { &*(self.transmit_ring.as_ptr().add(self.transmit_clean_index) as *const Td) }; @@ -180,9 +184,11 @@ impl NetworkAdapter for Intel8254x { } let desc = - unsafe { &mut *(self.transmit_ring.as_ptr().add(self.transmit_index) as *mut Td) }; + // SAFETY: caller must verify the safety contract for this operation +unsafe { &mut *(self.transmit_ring.as_ptr().add(self.transmit_index) as *mut Td) }; - let data = unsafe { + let data = // SAFETY: caller must verify the safety contract for this operation +unsafe { slice::from_raw_parts_mut( self.transmit_buffer[self.transmit_index].as_ptr() as *mut u8, cmp::min(buf.len(), self.transmit_buffer[self.transmit_index].len()) as usize, @@ -207,7 +213,8 @@ impl NetworkAdapter for Intel8254x { self.transmit_ring_free -= 1; self.transmit_buffer[self.transmit_index].sync_for_device(); - unsafe { self.write_reg(TDT, self.transmit_index as u32) }; + // SAFETY: caller guarantees pointer is valid, aligned, and live +unsafe { self.write_reg(TDT, self.transmit_index as u32) }; Ok(i) } @@ -215,7 +222,8 @@ impl NetworkAdapter for Intel8254x { fn dma_array() -> Result<[Dma; N]> { let vec: Vec> = (0..N) - .map(|_| Ok(unsafe { Dma::zeroed()?.assume_init() })) + .map(|_| Ok(// SAFETY: caller must verify the safety contract for this operation +unsafe { Dma::zeroed()?.assume_init() })) .collect::>>()?; vec.try_into().map_err(|_| Error::new(EIO)) } diff --git a/drivers/net/ixgbed/src/device.rs b/drivers/net/ixgbed/src/device.rs index 5f48d363a7..b5d3f911a8 100644 --- a/drivers/net/ixgbed/src/device.rs +++ b/drivers/net/ixgbed/src/device.rs @@ -37,14 +37,16 @@ impl NetworkAdapter for Intel8259x { } fn read_packet(&mut self, buf: &mut [u8]) -> Result> { - let desc = unsafe { + let desc = // SAFETY: caller must verify the safety contract for this operation +unsafe { &mut *(self.receive_ring.as_ptr().add(self.receive_index) as *mut ixgbe_adv_rx_desc) }; // Acquire fence pairs the device's write of status / // length with our subsequent reads. core::sync::atomic::fence(core::sync::atomic::Ordering::Acquire); - let status = unsafe { desc.wb.upper.status_error }; + let status = // SAFETY: caller must verify the safety contract for this operation +unsafe { desc.wb.upper.status_error }; if (status & IXGBE_RXDADV_STAT_DD) != 0 { if (status & IXGBE_RXDADV_STAT_EOP) == 0 { @@ -56,7 +58,8 @@ impl NetworkAdapter for Intel8259x { return Ok(None); } - let data = unsafe { + let data = // SAFETY: caller must verify the safety contract for this operation +unsafe { &self.receive_buffer[self.receive_index][..desc.wb.upper.length as usize] }; @@ -82,12 +85,14 @@ impl NetworkAdapter for Intel8259x { fn write_packet(&mut self, buf: &[u8]) -> Result { if self.transmit_ring_free == 0 { loop { - let desc = unsafe { + let desc = // SAFETY: caller must verify the safety contract for this operation +unsafe { &*(self.transmit_ring.as_ptr().add(self.transmit_clean_index) as *const ixgbe_adv_tx_desc) }; - if (unsafe { desc.wb.status } & IXGBE_ADVTXD_STAT_DD) != 0 { + if (// SAFETY: caller must verify the safety contract for this operation +unsafe { desc.wb.status } & IXGBE_ADVTXD_STAT_DD) != 0 { self.transmit_clean_index = wrap_ring(self.transmit_clean_index, self.transmit_ring.len()); self.transmit_ring_free += 1; @@ -101,11 +106,13 @@ impl NetworkAdapter for Intel8259x { } } - let desc = unsafe { + let desc = // SAFETY: caller must verify the safety contract for this operation +unsafe { &mut *(self.transmit_ring.as_ptr().add(self.transmit_index) as *mut ixgbe_adv_tx_desc) }; - let data = unsafe { + let data = // SAFETY: caller must verify the safety contract for this operation +unsafe { slice::from_raw_parts_mut( self.transmit_buffer[self.transmit_index].as_ptr() as *mut u8, cmp::min(buf.len(), self.transmit_buffer[self.transmit_index].len()) as usize, @@ -144,7 +151,8 @@ impl Intel8259x { base, size, receive_buffer: (0..32) - .map(|_| Ok(unsafe { Dma::zeroed()?.assume_init() })) + .map(|_| Ok(// SAFETY: caller must verify the safety contract for this operation +unsafe { Dma::zeroed()?.assume_init() })) .collect::>>()? .try_into() .map_err(|v: Vec<_>| { @@ -154,9 +162,11 @@ impl Intel8259x { ); Error::new(EIO) })?, - receive_ring: unsafe { Dma::zeroed()?.assume_init() }, + receive_ring: // SAFETY: caller must verify the safety contract for this operation +unsafe { Dma::zeroed()?.assume_init() }, transmit_buffer: (0..32) - .map(|_| Ok(unsafe { Dma::zeroed()?.assume_init() })) + .map(|_| Ok(// SAFETY: caller must verify the safety contract for this operation +unsafe { Dma::zeroed()?.assume_init() })) .collect::>>()? .try_into() .map_err(|v: Vec<_>| { @@ -167,7 +177,8 @@ impl Intel8259x { Error::new(EIO) })?, receive_index: 0, - transmit_ring: unsafe { Dma::zeroed()?.assume_init() }, + transmit_ring: // SAFETY: caller must verify the safety contract for this operation +unsafe { Dma::zeroed()?.assume_init() }, transmit_ring_free: 32, transmit_index: 0, transmit_clean_index: 0, @@ -185,18 +196,21 @@ impl Intel8259x { } pub fn next_read(&self) -> usize { - let desc = unsafe { + let desc = // SAFETY: caller must verify the safety contract for this operation +unsafe { &*(self.receive_ring.as_ptr().add(self.receive_index) as *const ixgbe_adv_rx_desc) }; - let status = unsafe { desc.wb.upper.status_error }; + let status = // SAFETY: caller must verify the safety contract for this operation +unsafe { desc.wb.upper.status_error }; if (status & IXGBE_RXDADV_STAT_DD) != 0 { if (status & IXGBE_RXDADV_STAT_EOP) == 0 { log::error!("ixgbed: received fragmented packet, buffer too small"); } - return unsafe { desc.wb.upper.length as usize }; + return // SAFETY: caller must verify the safety contract for this operation +unsafe { desc.wb.upper.length as usize }; } 0 @@ -238,7 +252,8 @@ impl Intel8259x { "MMIO access out of bounds" ); - unsafe { ptr::read_volatile((self.base + register as usize) as *mut u32) } + // SAFETY: caller guarantees pointer is valid, aligned, and live +unsafe { ptr::read_volatile((self.base + register as usize) as *mut u32) } } fn write_reg(&self, register: u32, data: u32) -> u32 { @@ -247,7 +262,8 @@ impl Intel8259x { "MMIO access out of bounds" ); - unsafe { + // SAFETY: caller must verify the safety contract for this operation +unsafe { ptr::write_volatile((self.base + register as usize) as *mut u32, data); ptr::read_volatile((self.base + register as usize) as *mut u32) } diff --git a/drivers/net/virtio-netd/src/main.rs b/drivers/net/virtio-netd/src/main.rs index 388447630d..e51e398cb9 100644 --- a/drivers/net/virtio-netd/src/main.rs +++ b/drivers/net/virtio-netd/src/main.rs @@ -60,7 +60,8 @@ fn deamon( // Negotiate device features: let mac_address = if device.transport.check_device_feature(VIRTIO_NET_F_MAC) { - let mac = unsafe { + let mac = // SAFETY: caller must verify the safety contract for this operation +unsafe { [ core::ptr::read_volatile(device_space.add(0)), core::ptr::read_volatile(device_space.add(1)), diff --git a/drivers/net/virtio-netd/src/scheme.rs b/drivers/net/virtio-netd/src/scheme.rs index 6553ecdf50..d06fda6921 100644 --- a/drivers/net/virtio-netd/src/scheme.rs +++ b/drivers/net/virtio-netd/src/scheme.rs @@ -27,7 +27,8 @@ impl<'a> VirtioNet<'a> { // Populate all of the `rx_queue` with buffers to maximize performence. let mut rx_buffers = vec![]; for i in 0..(rx.descriptor_len() as usize) { - let buf = unsafe { + let buf = // SAFETY: caller must verify the safety contract for this operation +unsafe { match Dma::<[u8]>::zeroed_slice(MAX_BUFFER_LEN) { Ok(dma) => dma.assume_init(), Err(err) => { @@ -75,7 +76,8 @@ impl<'a> VirtioNet<'a> { // and the device is notified of the new entry (see 5.1.5 Device Initialization). let buffer = &self.rx_buffers[descriptor_idx as usize]; buffer.sync_for_cpu(); - let header = unsafe { &*(buffer.as_ptr() as *const VirtHeader) }; + let header = // SAFETY: caller must verify the safety contract for this operation +unsafe { &*(buffer.as_ptr() as *const VirtHeader) }; let header_flags = header.flags; let packet = &buffer[header_size..(header_size + payload_size)]; @@ -109,9 +111,11 @@ impl<'a> NetworkAdapter for VirtioNet<'a> { } fn write_packet(&mut self, buffer: &[u8]) -> syscall::Result { - let header = unsafe { Dma::::zeroed()?.assume_init() }; + let header = // SAFETY: caller must verify the safety contract for this operation +unsafe { Dma::::zeroed()?.assume_init() }; - let mut payload = unsafe { Dma::<[u8]>::zeroed_slice(buffer.len())?.assume_init() }; + let mut payload = // SAFETY: caller must verify the safety contract for this operation +unsafe { Dma::<[u8]>::zeroed_slice(buffer.len())?.assume_init() }; payload.copy_from_slice(buffer); payload.sync_for_device(); diff --git a/netstack/src/buffer_pool.rs b/netstack/src/buffer_pool.rs index 5991f9ece3..f4f525bec0 100644 --- a/netstack/src/buffer_pool.rs +++ b/netstack/src/buffer_pool.rs @@ -81,7 +81,8 @@ impl BufferPool { Some(mut v) => { // memsetting the buffer with `resize` would be a waste of time let capacity = v.capacity(); - unsafe { + // SAFETY: caller must verify the safety contract for this operation +unsafe { v.set_len(capacity); } v diff --git a/netstack/src/scheme/mod.rs b/netstack/src/scheme/mod.rs index fa4dae29d2..18e2cc2a8e 100644 --- a/netstack/src/scheme/mod.rs +++ b/netstack/src/scheme/mod.rs @@ -178,7 +178,8 @@ impl Smolnetd { } else { name }; - let mut link = EthernetLink::new(&dev_name, unsafe { + let mut link = EthernetLink::new(&dev_name, // SAFETY: caller must verify the safety contract for this operation +unsafe { File::from_raw_fd(nf.into_raw() as RawFd) }); link.set_mac_address(hw_addr); @@ -190,7 +191,8 @@ impl Smolnetd { router_device: network_device, socket_set: Rc::clone(&socket_set), timer: ::std::time::Instant::now(), - time_file: unsafe { File::from_raw_fd(time_file.into_raw() as RawFd) }, + time_file: // SAFETY: caller guarantees fd is valid, open, and not aliased +unsafe { File::from_raw_fd(time_file.into_raw() as RawFd) }, ip_scheme: IpScheme::new( "ip", Rc::clone(&iface), diff --git a/netstack/src/scheme/tcp.rs b/netstack/src/scheme/tcp.rs index 94d5b21a12..1430a92a8b 100644 --- a/netstack/src/scheme/tcp.rs +++ b/netstack/src/scheme/tcp.rs @@ -500,7 +500,8 @@ impl<'a> SchemeSocket for TcpSocket<'a> { tcpi_rcv_wnd: self.recv_capacity() as u32, tcpi_snd_mss: 1460, }; - let bytes = unsafe { + let bytes = // SAFETY: caller must verify the safety contract for this operation +unsafe { core::slice::from_raw_parts( &info as *const TcpInfo as *const u8, core::mem::size_of::(), @@ -541,7 +542,8 @@ impl<'a> SchemeSocket for TcpSocket<'a> { SO_LINGER => { // struct linger: l_onoff (4 bytes) + l_linger (4 bytes) let vals = [1i32, 0i32]; // on, 0s linger - let bytes = unsafe { + let bytes = // SAFETY: caller must verify the safety contract for this operation +unsafe { core::slice::from_raw_parts(vals.as_ptr() as *const u8, 8) }; let len = buf.len().min(bytes.len()); diff --git a/netstack/src/worker_pool.rs b/netstack/src/worker_pool.rs index b2bf014434..1910fcca72 100644 --- a/netstack/src/worker_pool.rs +++ b/netstack/src/worker_pool.rs @@ -58,11 +58,13 @@ impl OwnedFd { // SAFETY: the caller must guarantee that the raw fd is // valid and that we have exclusive ownership. We do not // own the original fd; the worker thread owns the dup. - let dup_fd = unsafe { libc::dup(raw as i32) }; + let dup_fd = // SAFETY: caller must verify the safety contract for this operation +unsafe { libc::dup(raw as i32) }; if dup_fd < 0 { return Err(std::io::Error::last_os_error()); } - let f = unsafe { File::from_raw_fd(dup_fd) }; + let f = // SAFETY: caller guarantees fd is valid, open, and not aliased +unsafe { File::from_raw_fd(dup_fd) }; Ok(Self { inner: f }) } }