Minor cleanup

CHANGELOG.md

@@ -8,6 +8,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- next-header -->
 ## [Unreleased] - ReleaseDate
+### Fixed
+- [PR#20](https://github.com/Jake-Shadle/xdp/pull/20) changed `EtherType` and `IpProto` from enums to scoped constants to avoid UB in the presence of invalid/corrupt data that didn't match a variant. Also removed a bunch of the `IpProto` variants as most will never be used, and since it's now scoped constants users can provide their own constants without needing them in the lib themselves. Resolved [#19](https://github.com/Jake-Shadle/xdp/issues/19).
+- [PR#23](https://github.com/Jake-Shadle/xdp/pull/23) added sanity checks to avoid subtraction underflow if the user provides wildly out of range offsets and/or slices to `Packet` methods.
+- [PR#23](https://github.com/Jake-Shadle/xdp/pull/23) fixed a bug in `Packet::array_at_offset` where the offset was incorrect if `head` was not 0.
+- [PR#23](https://github.com/Jake-Shadle/xdp/pull/23) added a check in `UdpHeader::parse_packet` to ensure the UDP length matches the packet buffer length.
+
+### Changed
+- [PR#22](https://github.com/Jake-Shadle/xdp/pull/22) removed the `Index/Mut` impls from `XskProducer/Consumer` as they were unneccessary fluff in favor of much simpler internal methods.
+- [PR#23](https://github.com/Jake-Shadle/xdp/pull/23) changed `data_offset` and `data_length` to just `data`, a range that is convertible to/from `std::ops::Range<usize>`. `data_length` is now a method that just returns `data.end - data.start`.
+
+### Added
+- [PR#23](https://github.com/Jake-Shadle/xdp/pull/23) added `Packet::append` as a simpler way to add data to the tail of the packet.
+- [PR#23](https://github.com/Jake-Shadle/xdp/pull/23) added `csum::DataChecksum` as a simpler way to calculate the checksum of the data portion of a payload. `UdpHeaders::calc_checksum` now uses this instead of separate length and checksum arguments.
+
 ## [0.6.0] - 2025-03-04
 ### Changed
 - [PR#16](https://github.com/Jake-Shadle/xdp/pull/16) changed `RxRing` and `TxRing` to use the new `slab::Slab` trait.

crates/integ/tests/tx_checksum.rs

@@ -121,36 +121,34 @@ fn do_checksum_test(software: bool, vpair: &VethPair) {
                 .expect("not a UDP packet");
 
             // For this packet, we calculate the full checksum
-            packet.adjust_tail(-(udp.data_length as i32)).unwrap();
-            packet.insert(udp.data_offset, serverp).unwrap();
+            packet.adjust_tail(-(udp.data_length() as i32)).unwrap();
+            packet.insert(udp.data.start, serverp).unwrap();
 
             let nt::IpHdr::V4(mut copy) = udp.ip else {
                 unreachable!()
             };
             std::mem::swap(&mut copy.destination, &mut copy.source);
             copy.time_to_live -= 1;
 
-            let mut new = nt::UdpHeaders {
-                eth: nt::EthHdr {
+            let mut new = nt::UdpHeaders::new(
+                nt::EthHdr {
                     source: udp.eth.destination,
                     destination: udp.eth.source,
                     ether_type: udp.eth.ether_type,
                 },
-                ip: nt::IpHdr::V4(copy),
-                udp: nt::UdpHdr {
+                nt::IpHdr::V4(copy),
+                nt::UdpHdr {
                     destination: udp.udp.source,
                     source: sport.into(),
                     length: 0.into(),
                     check: 0,
                 },
-                data_offset: udp.data_offset,
-                data_length: serverp.len(),
-            };
+                udp.data.start..udp.data.start + serverp.len(),
+            );
 
             // For this packet, we calculate the full checksum
-            let data_checksum = csum::partial(serverp, 0);
-            let full_checksum = new.calc_checksum(serverp.len(), data_checksum);
-            new.set_packet_headers(&mut packet, true).unwrap();
+            let full_checksum = new.calc_checksum(csum::DataChecksum::calculate(serverp));
+            new.set_packet_headers(&mut packet).unwrap();
             println!("Full checksum: {full_checksum:04x}");
 
             slab.push_front(packet);
@@ -175,32 +173,31 @@ fn do_checksum_test(software: bool, vpair: &VethPair) {
                 .expect("failed to parse packet")
                 .expect("not a UDP packet");
 
-            packet.adjust_tail(-(udp.data_length as i32)).unwrap();
-            packet.insert(udp.data_offset, serverp).unwrap();
+            packet.adjust_tail(-(udp.data_length() as i32)).unwrap();
+            packet.insert(udp.data.start, serverp).unwrap();
 
             let nt::IpHdr::V4(mut copy) = udp.ip else {
                 unreachable!()
             };
             std::mem::swap(&mut copy.destination, &mut copy.source);
             copy.time_to_live -= 1;
 
-            let mut new = nt::UdpHeaders {
-                eth: nt::EthHdr {
+            let mut new = nt::UdpHeaders::new(
+                nt::EthHdr {
                     source: udp.eth.destination,
                     destination: udp.eth.source,
                     ether_type: udp.eth.ether_type,
                 },
-                ip: nt::IpHdr::V4(copy),
-                udp: nt::UdpHdr {
+                nt::IpHdr::V4(copy),
+                nt::UdpHdr {
                     destination: udp.udp.source,
                     source: sport.into(),
                     length: 0.into(),
                     check: 0,
                 },
-                data_offset: udp.data_offset,
-                data_length: serverp.len(),
-            };
-            new.set_packet_headers(&mut packet, true).unwrap();
+                udp.data.start..udp.data.start + serverp.len(),
+            );
+            new.set_packet_headers(&mut packet).unwrap();
             println!(
                 "partial checksum: {:04x}",
                 packet.calc_udp_checksum().unwrap()

crates/tests/tests/csum.rs

@@ -3,7 +3,7 @@
 use etherparse::PacketBuilder;
 use std::net::*;
 use tests::*;
-use xdp::packet::{net_types::*, *};
+use xdp::packet::{csum, net_types::*, *};
 
 /// Ensures we generate the correct IPv4 header checksum
 #[test]
@@ -80,8 +80,7 @@ fn combines_partial_checksums() {
         let expected = udp.udp.check;
         assert_eq!(packet.calc_udp_checksum().unwrap(), expected);
 
-        let data_checksum = csum::partial(LARGER, 0);
-        udp.calc_checksum(LARGER.len(), data_checksum);
+        udp.calc_checksum(csum::DataChecksum::calculate(LARGER));
         assert_eq!(udp.udp.check, expected);
     }
 
@@ -101,8 +100,7 @@ fn combines_partial_checksums() {
         let expected = udp.udp.check;
         assert_eq!(packet.calc_udp_checksum().unwrap(), expected);
 
-        let data_checksum = csum::partial(LARGER, 0);
-        udp.calc_checksum(LARGER.len(), data_checksum);
+        udp.calc_checksum(csum::DataChecksum::calculate(LARGER));
         assert_eq!(udp.udp.check, expected);
     }
 }

crates/tests/tests/packet.rs

@@ -31,6 +31,9 @@ fn simple() {
         tot_len - xdp::libc::xdp::XDP_PACKET_HEADROOM as usize,
     );
 
+    packet.adjust_tail(21).unwrap();
+    packet.adjust_head(21).unwrap();
+
     let val = b"deadbeef";
 
     packet.insert(0, val).unwrap();
@@ -72,6 +75,18 @@ fn simple() {
 
     packet.adjust_tail(-(packet.len() as i32)).unwrap();
     assert!(packet.is_empty());
+
+    packet
+        .insert(0, &0xf3f3f3f3f3f3f3f3u64.to_ne_bytes())
+        .unwrap();
+    packet.append(&0x1212121212121212u64.to_ne_bytes()).unwrap();
+
+    assert_eq!(packet.len(), 16);
+    let mut arr6 = [0u8; 8];
+    packet.array_at_offset(0, &mut arr6).unwrap();
+    assert_eq!(0xf3f3f3f3f3f3f3f3, u64::from_ne_bytes(arr6));
+    packet.array_at_offset(8, &mut arr6).unwrap();
+    assert_eq!(0x1212121212121212, u64::from_ne_bytes(arr6));
 }
 
 #[test]
@@ -125,25 +140,26 @@ fn udp_send() {
     ipv6.reset(64, nt::IpProto::Udp);
     ipv6.source = [10; 16];
     ipv6.destination = [1; 16];
-    let mut udp = UdpHeaders {
-        eth: nt::EthHdr {
+    let data_offset = nt::EthHdr::LEN + nt::Ipv6Hdr::LEN + nt::UdpHdr::LEN;
+
+    let mut udp = UdpHeaders::new(
+        nt::EthHdr {
             source: MacAddress([1; 6]),
             destination: MacAddress([2; 6]),
             ether_type: nt::EtherType::Ipv6,
         },
-        ip: nt::IpHdr::V6(ipv6),
-        udp: nt::UdpHdr {
+        nt::IpHdr::V6(ipv6),
+        nt::UdpHdr {
             source: 8900.into(),
             destination: 9001.into(),
             length: 0.into(),
             check: 0,
         },
-        data_offset: nt::EthHdr::LEN + nt::Ipv6Hdr::LEN + nt::UdpHdr::LEN,
-        data_length: payload.len(),
-    };
+        data_offset..data_offset + payload.len(),
+    );
 
-    udp.set_packet_headers(&mut packet, false).unwrap();
-    packet.insert(udp.data_offset, &payload).unwrap();
+    udp.set_packet_headers(&mut packet).unwrap();
+    packet.insert(udp.data.start, &payload).unwrap();
 
     let check = packet.calc_udp_checksum().unwrap();
 
@@ -213,10 +229,7 @@ fn parses_ipv4() {
             destination: Ipv4Addr::new(192, 168, 1, 1),
         }
     );
-    assert_eq!(
-        &packet[udp.data_offset..udp.data_offset + udp.data_length],
-        IPV4_DATA
-    );
+    assert_eq!(&packet[udp.data], IPV4_DATA);
 }
 
 /// Ensures we can parse an IPv6 UDP packet
@@ -246,8 +259,71 @@ fn parses_ipv6() {
             destination: DST,
         }
     );
+    assert_eq!(&packet[udp.data], IPV6_DATA);
+}
+
+/// Ensures the UDP length field matches the packet
+#[test]
+fn rejects_invalid_udp_length() {
+    let mut buf = [0u8; 2048];
+    let mut packet = Packet::testing_new(&mut buf);
+
+    PacketBuilder::ethernet2(SRC_MAC.0, DST_MAC.0)
+        .ipv6([20; 16], [33; 16], 64)
+        .udp(5353, 1111)
+        .write(&mut packet, IPV6_DATA)
+        .unwrap();
+
+    let mut udp_hdr: nt::UdpHdr = packet.read(nt::EthHdr::LEN + nt::Ipv6Hdr::LEN).unwrap();
+    assert_eq!(udp_hdr.source.host(), 5353);
+    assert_eq!(udp_hdr.destination.host(), 1111);
     assert_eq!(
-        &packet[udp.data_offset..udp.data_offset + udp.data_length],
-        IPV6_DATA
+        udp_hdr.length.host() as usize,
+        IPV6_DATA.len() + nt::UdpHdr::LEN
     );
+
+    // too long
+    {
+        udp_hdr.length = u16::MAX.into();
+        packet
+            .write(nt::EthHdr::LEN + nt::Ipv6Hdr::LEN, udp_hdr)
+            .unwrap();
+        assert!(UdpHeaders::parse_packet(&packet).is_err());
+    }
+
+    // too short
+    {
+        udp_hdr.length = (nt::UdpHdr::LEN as u16).into();
+        packet
+            .write(nt::EthHdr::LEN + nt::Ipv6Hdr::LEN, udp_hdr)
+            .unwrap();
+        assert!(UdpHeaders::parse_packet(&packet).is_err());
+    }
+
+    // off by 1
+    {
+        udp_hdr.length = ((nt::UdpHdr::LEN + IPV6_DATA.len() + 1) as u16).into();
+        packet
+            .write(nt::EthHdr::LEN + nt::Ipv6Hdr::LEN, udp_hdr)
+            .unwrap();
+        assert!(UdpHeaders::parse_packet(&packet).is_err());
+    }
+}
+
+#[test]
+#[should_panic]
+fn data_range() {
+    let mut buf = [0u8; 2 * 1024];
+    let mut packet = Packet::testing_new(&mut buf);
+
+    const DATA: &[u8] = &[0x43; 31];
+
+    packet.append(DATA).unwrap();
+    assert_eq!(packet.len(), DATA.len());
+
+    let mut range: xdp::packet::net_types::DataRange = (0..DATA.len()).into();
+    assert_eq!(&packet[range], DATA);
+
+    range.start = range.end + 1;
+    dbg!(&packet[range]);
 }