Merge #535

535: Fuzz fixes r=Dirbaio a=Dirbaio



Co-authored-by: Dario Nieuwenhuis <dirbaio@dirbaio.net>

examples/utils.rs

@@ -5,12 +5,10 @@ use env_logger::Builder;
 use getopts::{Matches, Options};
 #[cfg(feature = "log")]
 use log::{trace, Level, LevelFilter};
-use std::cell::RefCell;
 use std::env;
 use std::fs::File;
 use std::io::{self, Write};
 use std::process;
-use std::rc::Rc;
 use std::str::{self, FromStr};
 use std::time::{SystemTime, UNIX_EPOCH};
 
@@ -18,7 +16,7 @@ use smoltcp::phy::RawSocket;
 #[cfg(feature = "phy-tuntap_interface")]
 use smoltcp::phy::TunTapInterface;
 use smoltcp::phy::{Device, FaultInjector, Medium, Tracer};
-use smoltcp::phy::{PcapMode, PcapSink, PcapWriter};
+use smoltcp::phy::{PcapMode, PcapWriter};
 use smoltcp::time::{Duration, Instant};
 
 #[cfg(feature = "log")]
@@ -165,7 +163,7 @@ pub fn parse_middleware_options<D>(
     matches: &mut Matches,
     device: D,
     loopback: bool,
-) -> FaultInjector<Tracer<PcapWriter<D, Rc<dyn PcapSink>>>>
+) -> FaultInjector<Tracer<PcapWriter<D, Box<dyn io::Write>>>>
 where
     D: for<'a> Device<'a>,
 {
@@ -208,7 +206,7 @@ where
 
     let device = PcapWriter::new(
         device,
-        Rc::new(RefCell::new(pcap_writer)) as Rc<dyn PcapSink>,
+        pcap_writer,
         if loopback {
             PcapMode::TxOnly
         } else {

fuzz/Cargo.toml

@@ -3,21 +3,15 @@ name = "smoltcp-fuzz"
 version = "0.0.1"
 authors = ["Automatically generated"]
 publish = false
+edition = "2018"
 
 [package.metadata]
 cargo-fuzz = true
 
 [dependencies]
+libfuzzer-sys = "0.4"
 getopts = "0.2"
-
-[dependencies.smoltcp]
-path = ".."
-
-[dependencies.libfuzzer-sys]
-git = "https://github.com/rust-fuzz/libfuzzer-sys.git"
-
-[profile.release]
-codegen-units = 1 # needed to prevent weird linker error about sancov guards
+smoltcp = { path = "..", features = [ "medium-ethernet" ] }
 
 # Prevent this from interfering with workspaces
 [workspace]
@@ -26,7 +20,11 @@ members = ["."]
 [[bin]]
 name = "packet_parser"
 path = "fuzz_targets/packet_parser.rs"
+test = false
+doc = false
 
 [[bin]]
 name = "tcp_headers"
 path = "fuzz_targets/tcp_headers.rs"
+test = false
+doc = false

fuzz/fuzz_targets/packet_parser.rs

@@ -1,8 +1,10 @@
 #![no_main]
-#[macro_use] extern crate libfuzzer_sys;
-extern crate smoltcp;
+use libfuzzer_sys::fuzz_target;
+use smoltcp::wire::*;
 
 fuzz_target!(|data: &[u8]| {
-    use smoltcp::wire::*;
-    format!("{}", PrettyPrinter::<EthernetFrame<&'static [u8]>>::new("", &data));
+    format!(
+        "{}",
+        PrettyPrinter::<EthernetFrame<&'static [u8]>>::new("", &data)
+    );
 });

fuzz/fuzz_targets/tcp_headers.rs

@@ -1,26 +1,20 @@
 #![no_main]
-#[macro_use] extern crate libfuzzer_sys;
-extern crate smoltcp;
-
-use std as core;
-extern crate getopts;
-
-use core::cmp;
+use libfuzzer_sys::fuzz_target;
+use smoltcp::iface::{InterfaceBuilder, NeighborCache};
 use smoltcp::phy::{Loopback, Medium};
-use smoltcp::wire::{EthernetAddress, EthernetFrame, EthernetProtocol};
-use smoltcp::wire::{IpAddress, IpCidr, Ipv4Packet, Ipv6Packet, TcpPacket};
-use smoltcp::iface::{NeighborCache, InterfaceBuilder};
 use smoltcp::socket::{SocketSet, TcpSocket, TcpSocketBuffer};
 use smoltcp::time::{Duration, Instant};
+use smoltcp::wire::{EthernetAddress, EthernetFrame, EthernetProtocol};
+use smoltcp::wire::{IpAddress, IpCidr, Ipv4Packet, Ipv6Packet, TcpPacket};
+use std::cmp;
 
-mod utils {
-    include!("../utils.rs");
-}
+#[path = "../utils.rs"]
+mod utils;
 
 mod mock {
+    use smoltcp::time::{Duration, Instant};
+    use std::sync::atomic::{AtomicUsize, Ordering};
     use std::sync::Arc;
-    use std::sync::atomic::{Ordering, AtomicUsize};
-	use smoltcp::time::{Duration, Instant};
 
     // should be AtomicU64 but that's unstable
     #[derive(Debug, Clone)]
@@ -33,7 +27,8 @@ mod mock {
         }
 
         pub fn advance(&self, duration: Duration) {
-            self.0.fetch_add(duration.total_millis() as usize, Ordering::SeqCst);
+            self.0
+                .fetch_add(duration.total_millis() as usize, Ordering::SeqCst);
         }
 
         pub fn elapsed(&self) -> Instant {
@@ -52,7 +47,10 @@ impl TcpHeaderFuzzer {
     //
     // Otherwise, it replaces the entire rest of the TCP header with the fuzzer's output.
     pub fn new(data: &[u8]) -> TcpHeaderFuzzer {
-        let copy_len = cmp::min(data.len(), 56 /* max TCP header length without port numbers*/);
+        let copy_len = cmp::min(
+            data.len(),
+            56, /* max TCP header length without port numbers*/
+        );
 
         let mut fuzzer = TcpHeaderFuzzer([0; 56], copy_len);
         fuzzer.0[..copy_len].copy_from_slice(&data[..copy_len]);
@@ -68,13 +66,16 @@ impl smoltcp::phy::Fuzzer for TcpHeaderFuzzer {
 
         let tcp_packet_offset = {
             let eth_frame = EthernetFrame::new_unchecked(&frame_data);
-            EthernetFrame::<&mut [u8]>::header_len() + match eth_frame.ethertype() {
-                EthernetProtocol::Ipv4 =>
-                    Ipv4Packet::new_unchecked(eth_frame.payload()).header_len() as usize,
-                EthernetProtocol::Ipv6 =>
-                    Ipv6Packet::new_unchecked(eth_frame.payload()).header_len() as usize,
-                _ => return
-            }
+            EthernetFrame::<&mut [u8]>::header_len()
+                + match eth_frame.ethertype() {
+                    EthernetProtocol::Ipv4 => {
+                        Ipv4Packet::new_unchecked(eth_frame.payload()).header_len() as usize
+                    }
+                    EthernetProtocol::Ipv6 => {
+                        Ipv6Packet::new_unchecked(eth_frame.payload()).header_len() as usize
+                    }
+                    _ => return,
+                }
         };
 
         let tcp_is_syn = {
@@ -95,7 +96,7 @@ impl smoltcp::phy::Fuzzer for TcpHeaderFuzzer {
             (tcp_packet[12] as usize >> 4) * 4
         };
 
-        let tcp_packet = &mut frame_data[tcp_packet_offset+4..];
+        let tcp_packet = &mut frame_data[tcp_packet_offset + 4..];
 
         let replacement_data = &self.0[..self.1];
         let copy_len = cmp::min(replacement_data.len(), tcp_header_len);
@@ -114,17 +115,17 @@ fuzz_target!(|data: &[u8]| {
     let clock = mock::Clock::new();
 
     let device = {
-
         let (mut opts, mut free) = utils::create_options();
         utils::add_middleware_options(&mut opts, &mut free);
 
         let mut matches = utils::parse_options(&opts, free);
-        let device = utils::parse_middleware_options(&mut matches, Loopback::new(Medium::Ethernet),
-                                                     /*loopback=*/true);
+        let device = utils::parse_middleware_options(
+            &mut matches,
+            Loopback::new(Medium::Ethernet),
+            /*loopback=*/ true,
+        );
 
-        smoltcp::phy::FuzzInjector::new(device,
-                                        EmptyFuzzer(),
-                                        TcpHeaderFuzzer::new(data))
+        smoltcp::phy::FuzzInjector::new(device, EmptyFuzzer(), TcpHeaderFuzzer::new(data))
     };
 
     let mut neighbor_cache_entries = [None; 8];
@@ -132,10 +133,10 @@ fuzz_target!(|data: &[u8]| {
 
     let ip_addrs = [IpCidr::new(IpAddress::v4(127, 0, 0, 1), 8)];
     let mut iface = InterfaceBuilder::new(device)
-            .ethernet_addr(EthernetAddress::default())
-            .neighbor_cache(neighbor_cache)
-            .ip_addrs(ip_addrs)
-            .finalize();
+        .ethernet_addr(EthernetAddress::default())
+        .neighbor_cache(neighbor_cache)
+        .ip_addrs(ip_addrs)
+        .finalize();
 
     let server_socket = {
         // It is not strictly necessary to use a `static mut` and unsafe code here, but
@@ -162,7 +163,7 @@ fuzz_target!(|data: &[u8]| {
     let server_handle = socket_set.add(server_socket);
     let client_handle = socket_set.add(client_socket);
 
-    let mut did_listen  = false;
+    let mut did_listen = false;
     let mut did_connect = false;
     let mut done = false;
     while !done && clock.elapsed() < Instant::from_millis(4_000) {
@@ -187,24 +188,28 @@ fuzz_target!(|data: &[u8]| {
             let mut socket = socket_set.get::<TcpSocket>(client_handle);
             if !socket.is_open() {
                 if !did_connect {
-                    socket.connect((IpAddress::v4(127, 0, 0, 1), 1234),
-                                   (IpAddress::Unspecified, 65000)).unwrap();
+                    socket
+                        .connect(
+                            (IpAddress::v4(127, 0, 0, 1), 1234),
+                            (IpAddress::Unspecified, 65000),
+                        )
+                        .unwrap();
                     did_connect = true;
                 }
             }
 
             if socket.can_send() {
-                socket.send_slice(b"0123456789abcdef0123456789abcdef0123456789abcdef").unwrap();
+                socket
+                    .send_slice(b"0123456789abcdef0123456789abcdef0123456789abcdef")
+                    .unwrap();
                 socket.close();
             }
         }
 
         match iface.poll_delay(&socket_set, clock.elapsed()) {
-            Some(Duration { millis: 0 }) => {},
-            Some(delay) => {
-                clock.advance(delay)
-            },
-            None => clock.advance(Duration::from_millis(1))
+            Some(Duration { millis: 0 }) => {}
+            Some(delay) => clock.advance(delay),
+            None => clock.advance(Duration::from_millis(1)),
         }
     }
 });

fuzz/utils.rs

@@ -1,18 +1,17 @@
 // TODO: this is literally a copy of examples/utils.rs, but without an allow dead code attribute.
 // The include logic does not allow having attributes in included files.
 
-use std::cell::RefCell;
-use std::str::{self, FromStr};
-use std::rc::Rc;
-use std::io;
-use std::fs::File;
-use std::time::{SystemTime, UNIX_EPOCH};
+use getopts::{Matches, Options};
 use std::env;
+use std::fs::File;
+use std::io;
+use std::io::Write;
 use std::process;
-use getopts::{Options, Matches};
+use std::str::{self, FromStr};
+use std::time::{SystemTime, UNIX_EPOCH};
 
-use smoltcp::phy::{Device, EthernetTracer, FaultInjector};
-use smoltcp::phy::{PcapWriter, PcapSink, PcapMode, PcapLinkType};
+use smoltcp::phy::{Device, FaultInjector, Tracer};
+use smoltcp::phy::{PcapMode, PcapWriter};
 use smoltcp::time::Duration;
 
 pub fn create_options() -> (Options, Vec<&'static str>) {
@@ -29,10 +28,17 @@ pub fn parse_options(options: &Options, free: Vec<&str>) -> Matches {
         }
         Ok(matches) => {
             if matches.opt_present("h") || matches.free.len() != free.len() {
-                let brief = format!("Usage: {} [OPTION]... {}",
-                                    env::args().nth(0).unwrap(), free.join(" "));
+                let brief = format!(
+                    "Usage: {} [OPTION]... {}",
+                    env::args().nth(0).unwrap(),
+                    free.join(" ")
+                );
                 print!("{}", options.usage(&brief));
-                process::exit(if matches.free.len() != free.len() { 1 } else { 0 })
+                process::exit(if matches.free.len() != free.len() {
+                    1
+                } else {
+                    0
+                })
             }
             matches
         }
@@ -41,46 +47,102 @@ pub fn parse_options(options: &Options, free: Vec<&str>) -> Matches {
 
 pub fn add_middleware_options(opts: &mut Options, _free: &mut Vec<&str>) {
     opts.optopt("", "pcap", "Write a packet capture file", "FILE");
-    opts.optopt("", "drop-chance", "Chance of dropping a packet (%)", "CHANCE");
-    opts.optopt("", "corrupt-chance", "Chance of corrupting a packet (%)", "CHANCE");
-    opts.optopt("", "size-limit", "Drop packets larger than given size (octets)", "SIZE");
-    opts.optopt("", "tx-rate-limit", "Drop packets after transmit rate exceeds given limit \
-                                      (packets per interval)", "RATE");
-    opts.optopt("", "rx-rate-limit", "Drop packets after transmit rate exceeds given limit \
-                                      (packets per interval)", "RATE");
-    opts.optopt("", "shaping-interval", "Sets the interval for rate limiting (ms)", "RATE");
+    opts.optopt(
+        "",
+        "drop-chance",
+        "Chance of dropping a packet (%)",
+        "CHANCE",
+    );
+    opts.optopt(
+        "",
+        "corrupt-chance",
+        "Chance of corrupting a packet (%)",
+        "CHANCE",
+    );
+    opts.optopt(
+        "",
+        "size-limit",
+        "Drop packets larger than given size (octets)",
+        "SIZE",
+    );
+    opts.optopt(
+        "",
+        "tx-rate-limit",
+        "Drop packets after transmit rate exceeds given limit \
+                                      (packets per interval)",
+        "RATE",
+    );
+    opts.optopt(
+        "",
+        "rx-rate-limit",
+        "Drop packets after transmit rate exceeds given limit \
+                                      (packets per interval)",
+        "RATE",
+    );
+    opts.optopt(
+        "",
+        "shaping-interval",
+        "Sets the interval for rate limiting (ms)",
+        "RATE",
+    );
 }
 
-pub fn parse_middleware_options<D>(matches: &mut Matches, device: D, loopback: bool)
-        -> FaultInjector<EthernetTracer<PcapWriter<D, Rc<PcapSink>>>>
-    where D: for<'a> Device<'a>
+pub fn parse_middleware_options<D>(
+    matches: &mut Matches,
+    device: D,
+    loopback: bool,
+) -> FaultInjector<Tracer<PcapWriter<D, Box<dyn Write>>>>
+where
+    D: for<'a> Device<'a>,
 {
-    let drop_chance      = matches.opt_str("drop-chance").map(|s| u8::from_str(&s).unwrap())
-                                  .unwrap_or(0);
-    let corrupt_chance   = matches.opt_str("corrupt-chance").map(|s| u8::from_str(&s).unwrap())
-                                  .unwrap_or(0);
-    let size_limit       = matches.opt_str("size-limit").map(|s| usize::from_str(&s).unwrap())
-                                  .unwrap_or(0);
-    let tx_rate_limit    = matches.opt_str("tx-rate-limit").map(|s| u64::from_str(&s).unwrap())
-                                  .unwrap_or(0);
-    let rx_rate_limit    = matches.opt_str("rx-rate-limit").map(|s| u64::from_str(&s).unwrap())
-                                  .unwrap_or(0);
-    let shaping_interval = matches.opt_str("shaping-interval").map(|s| u64::from_str(&s).unwrap())
-                                  .unwrap_or(0);
+    let drop_chance = matches
+        .opt_str("drop-chance")
+        .map(|s| u8::from_str(&s).unwrap())
+        .unwrap_or(0);
+    let corrupt_chance = matches
+        .opt_str("corrupt-chance")
+        .map(|s| u8::from_str(&s).unwrap())
+        .unwrap_or(0);
+    let size_limit = matches
+        .opt_str("size-limit")
+        .map(|s| usize::from_str(&s).unwrap())
+        .unwrap_or(0);
+    let tx_rate_limit = matches
+        .opt_str("tx-rate-limit")
+        .map(|s| u64::from_str(&s).unwrap())
+        .unwrap_or(0);
+    let rx_rate_limit = matches
+        .opt_str("rx-rate-limit")
+        .map(|s| u64::from_str(&s).unwrap())
+        .unwrap_or(0);
+    let shaping_interval = matches
+        .opt_str("shaping-interval")
+        .map(|s| u64::from_str(&s).unwrap())
+        .unwrap_or(0);
 
-    let pcap_writer: Box<io::Write>;
+    let pcap_writer: Box<dyn io::Write>;
     if let Some(pcap_filename) = matches.opt_str("pcap") {
         pcap_writer = Box::new(File::create(pcap_filename).expect("cannot open file"))
     } else {
         pcap_writer = Box::new(io::sink())
     }
 
-    let seed = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().subsec_nanos();
+    let seed = SystemTime::now()
+        .duration_since(UNIX_EPOCH)
+        .unwrap()
+        .subsec_nanos();
+
+    let device = PcapWriter::new(
+        device,
+        pcap_writer,
+        if loopback {
+            PcapMode::TxOnly
+        } else {
+            PcapMode::Both
+        },
+    );
 
-    let device = PcapWriter::new(device, Rc::new(RefCell::new(pcap_writer)) as Rc<PcapSink>,
-                                 if loopback { PcapMode::TxOnly } else { PcapMode::Both },
-                                 PcapLinkType::Ethernet);
-    let device = EthernetTracer::new(device, |_timestamp, _printer| {
+    let device = Tracer::new(device, |_timestamp, _printer| {
         #[cfg(feature = "log")]
         trace!("{}", _printer);
     });

src/phy/fault_injector.rs

@@ -289,7 +289,7 @@ impl<'a, Rx: phy::RxToken> phy::RxToken for RxToken<'a, Rx> {
                 let mut corrupt = &mut corrupt[..buffer.len()];
                 corrupt.copy_from_slice(buffer);
                 state.borrow_mut().corrupt(&mut corrupt);
-                f(&mut corrupt)
+                f(corrupt)
             } else {
                 f(buffer)
             }

src/phy/fuzz_injector.rs

@@ -122,9 +122,10 @@ impl<'a, Tx: phy::TxToken, FTx: Fuzzer> phy::TxToken for TxToken<'a, Tx, FTx> {
         F: FnOnce(&mut [u8]) -> Result<R>,
     {
         let Self { fuzzer, token } = self;
-        token.consume(timestamp, len, |mut buf| {
-            fuzzer.fuzz_packet(&mut buf);
-            f(buf)
+        token.consume(timestamp, len, |buf| {
+            let result = f(buf);
+            fuzzer.fuzz_packet(buf);
+            result
         })
     }
 }

src/phy/pcap_writer.rs

@@ -1,8 +1,7 @@
 use byteorder::{ByteOrder, NativeEndian};
+use core::cell::RefCell;
 use phy::Medium;
 #[cfg(feature = "std")]
-use std::cell::RefCell;
-#[cfg(feature = "std")]
 use std::io::Write;
 
 use crate::phy::{self, Device, DeviceCapabilities};
@@ -34,17 +33,20 @@ pub enum PcapMode {
 /// A packet capture sink.
 pub trait PcapSink {
     /// Write data into the sink.
-    fn write(&self, data: &[u8]);
+    fn write(&mut self, data: &[u8]);
+
+    /// Flush data written into the sync.
+    fn flush(&mut self) {}
 
     /// Write an `u16` into the sink, in native byte order.
-    fn write_u16(&self, value: u16) {
+    fn write_u16(&mut self, value: u16) {
         let mut bytes = [0u8; 2];
         NativeEndian::write_u16(&mut bytes, value);
         self.write(&bytes[..])
     }
 
     /// Write an `u32` into the sink, in native byte order.
-    fn write_u32(&self, value: u32) {
+    fn write_u32(&mut self, value: u32) {
         let mut bytes = [0u8; 4];
         NativeEndian::write_u32(&mut bytes, value);
         self.write(&bytes[..])
@@ -53,7 +55,7 @@ pub trait PcapSink {
     /// Write the libpcap global header into the sink.
     ///
     /// This method may be overridden e.g. if special synchronization is necessary.
-    fn global_header(&self, link_type: PcapLinkType) {
+    fn global_header(&mut self, link_type: PcapLinkType) {
         self.write_u32(0xa1b2c3d4); // magic number
         self.write_u16(2); // major version
         self.write_u16(4); // minor version
@@ -69,7 +71,7 @@ pub trait PcapSink {
     ///
     /// # Panics
     /// This function panics if `length` is greater than 65535.
-    fn packet_header(&self, timestamp: Instant, length: usize) {
+    fn packet_header(&mut self, timestamp: Instant, length: usize) {
         assert!(length <= 65535);
 
         self.write_u32(timestamp.secs() as u32); // timestamp seconds
@@ -81,28 +83,21 @@ pub trait PcapSink {
     /// Write the libpcap packet header followed by packet data into the sink.
     ///
     /// See also the note for [global_header](#method.global_header).
-    fn packet(&self, timestamp: Instant, packet: &[u8]) {
+    fn packet(&mut self, timestamp: Instant, packet: &[u8]) {
         self.packet_header(timestamp, packet.len());
-        self.write(packet)
-    }
-}
-
-impl<T: AsRef<dyn PcapSink>> PcapSink for T {
-    fn write(&self, data: &[u8]) {
-        self.as_ref().write(data)
+        self.write(packet);
+        self.flush();
     }
 }
 
 #[cfg(feature = "std")]
-impl<T: Write> PcapSink for RefCell<T> {
-    fn write(&self, data: &[u8]) {
-        self.borrow_mut().write_all(data).expect("cannot write")
+impl<T: Write> PcapSink for T {
+    fn write(&mut self, data: &[u8]) {
+        T::write_all(self, data).expect("cannot write")
     }
 
-    fn packet(&self, timestamp: Instant, packet: &[u8]) {
-        self.packet_header(timestamp, packet.len());
-        PcapSink::write(self, packet);
-        self.borrow_mut().flush().expect("cannot flush")
+    fn flush(&mut self) {
+        T::flush(self).expect("cannot flush")
     }
 }
 
@@ -119,20 +114,19 @@ impl<T: Write> PcapSink for RefCell<T> {
 /// [libpcap]: https://wiki.wireshark.org/Development/LibpcapFileFormat
 /// [sink]: trait.PcapSink.html
 #[derive(Debug)]
-#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct PcapWriter<D, S>
 where
     D: for<'a> Device<'a>,
-    S: PcapSink + Clone,
+    S: PcapSink,
 {
     lower: D,
-    sink: S,
+    sink: RefCell<S>,
     mode: PcapMode,
 }
 
-impl<D: for<'a> Device<'a>, S: PcapSink + Clone> PcapWriter<D, S> {
+impl<D: for<'a> Device<'a>, S: PcapSink> PcapWriter<D, S> {
     /// Creates a packet capture writer.
-    pub fn new(lower: D, sink: S, mode: PcapMode) -> PcapWriter<D, S> {
+    pub fn new(lower: D, mut sink: S, mode: PcapMode) -> PcapWriter<D, S> {
         let medium = lower.capabilities().medium;
         let link_type = match medium {
             #[cfg(feature = "medium-ip")]
@@ -141,7 +135,11 @@ impl<D: for<'a> Device<'a>, S: PcapSink + Clone> PcapWriter<D, S> {
             Medium::Ethernet => PcapLinkType::Ethernet,
         };
         sink.global_header(link_type);
-        PcapWriter { lower, sink, mode }
+        PcapWriter {
+            lower,
+            sink: RefCell::new(sink),
+            mode,
+        }
     }
 
     /// Get a reference to the underlying device.
@@ -163,31 +161,27 @@ impl<D: for<'a> Device<'a>, S: PcapSink + Clone> PcapWriter<D, S> {
 impl<'a, D, S> Device<'a> for PcapWriter<D, S>
 where
     D: for<'b> Device<'b>,
-    S: PcapSink + Clone + 'a,
+    S: PcapSink + 'a,
 {
-    type RxToken = RxToken<<D as Device<'a>>::RxToken, S>;
-    type TxToken = TxToken<<D as Device<'a>>::TxToken, S>;
+    type RxToken = RxToken<'a, <D as Device<'a>>::RxToken, S>;
+    type TxToken = TxToken<'a, <D as Device<'a>>::TxToken, S>;
 
     fn capabilities(&self) -> DeviceCapabilities {
         self.lower.capabilities()
     }
 
     fn receive(&'a mut self) -> Option<(Self::RxToken, Self::TxToken)> {
-        let &mut Self {
-            ref mut lower,
-            ref sink,
-            mode,
-            ..
-        } = self;
-        lower.receive().map(|(rx_token, tx_token)| {
+        let sink = &self.sink;
+        let mode = self.mode;
+        self.lower.receive().map(move |(rx_token, tx_token)| {
             let rx = RxToken {
                 token: rx_token,
-                sink: sink.clone(),
+                sink,
                 mode,
             };
             let tx = TxToken {
                 token: tx_token,
-                sink: sink.clone(),
+                sink,
                 mode,
             };
             (rx, tx)
@@ -195,32 +189,29 @@ where
     }
 
     fn transmit(&'a mut self) -> Option<Self::TxToken> {
-        let &mut Self {
-            ref mut lower,
-            ref sink,
-            mode,
-        } = self;
-        lower.transmit().map(|token| TxToken {
-            token,
-            sink: sink.clone(),
-            mode,
-        })
+        let sink = &self.sink;
+        let mode = self.mode;
+        self.lower
+            .transmit()
+            .map(move |token| TxToken { token, sink, mode })
     }
 }
 
 #[doc(hidden)]
-pub struct RxToken<Rx: phy::RxToken, S: PcapSink> {
+pub struct RxToken<'a, Rx: phy::RxToken, S: PcapSink> {
     token: Rx,
-    sink: S,
+    sink: &'a RefCell<S>,
     mode: PcapMode,
 }
 
-impl<Rx: phy::RxToken, S: PcapSink> phy::RxToken for RxToken<Rx, S> {
+impl<'a, Rx: phy::RxToken, S: PcapSink> phy::RxToken for RxToken<'a, Rx, S> {
     fn consume<R, F: FnOnce(&mut [u8]) -> Result<R>>(self, timestamp: Instant, f: F) -> Result<R> {
         let Self { token, sink, mode } = self;
         token.consume(timestamp, |buffer| {
             match mode {
-                PcapMode::Both | PcapMode::RxOnly => sink.packet(timestamp, buffer.as_ref()),
+                PcapMode::Both | PcapMode::RxOnly => {
+                    sink.borrow_mut().packet(timestamp, buffer.as_ref())
+                }
                 PcapMode::TxOnly => (),
             }
             f(buffer)
@@ -229,13 +220,13 @@ impl<Rx: phy::RxToken, S: PcapSink> phy::RxToken for RxToken<Rx, S> {
 }
 
 #[doc(hidden)]
-pub struct TxToken<Tx: phy::TxToken, S: PcapSink> {
+pub struct TxToken<'a, Tx: phy::TxToken, S: PcapSink> {
     token: Tx,
-    sink: S,
+    sink: &'a RefCell<S>,
     mode: PcapMode,
 }
 
-impl<Tx: phy::TxToken, S: PcapSink> phy::TxToken for TxToken<Tx, S> {
+impl<'a, Tx: phy::TxToken, S: PcapSink> phy::TxToken for TxToken<'a, Tx, S> {
     fn consume<R, F>(self, timestamp: Instant, len: usize, f: F) -> Result<R>
     where
         F: FnOnce(&mut [u8]) -> Result<R>,
@@ -244,7 +235,7 @@ impl<Tx: phy::TxToken, S: PcapSink> phy::TxToken for TxToken<Tx, S> {
         token.consume(timestamp, len, |buffer| {
             let result = f(buffer);
             match mode {
-                PcapMode::Both | PcapMode::TxOnly => sink.packet(timestamp, buffer),
+                PcapMode::Both | PcapMode::TxOnly => sink.borrow_mut().packet(timestamp, buffer),
                 PcapMode::RxOnly => (),
             };
             result

src/socket/tcp.rs

@@ -188,15 +188,13 @@ enum Timer {
 const ACK_DELAY_DEFAULT: Duration = Duration { millis: 10 };
 const CLOSE_DELAY: Duration = Duration { millis: 10_000 };
 
-impl Default for Timer {
-    fn default() -> Timer {
+impl Timer {
+    fn new() -> Timer {
         Timer::Idle {
             keep_alive_at: None,
         }
     }
-}
 
-impl Timer {
     fn should_keep_alive(&self, timestamp: Instant) -> bool {
         match *self {
             Timer::Idle {
@@ -417,7 +415,7 @@ impl<'a> TcpSocket<'a> {
         TcpSocket {
             meta: SocketMeta::default(),
             state: State::Closed,
-            timer: Timer::default(),
+            timer: Timer::new(),
             rtte: RttEstimator::default(),
             assembler: Assembler::new(rx_buffer.capacity()),
             tx_buffer: tx_buffer,
@@ -644,7 +642,7 @@ impl<'a> TcpSocket<'a> {
             mem::size_of::<usize>() * 8 - self.rx_buffer.capacity().leading_zeros() as usize;
 
         self.state = State::Closed;
-        self.timer = Timer::default();
+        self.timer = Timer::new();
         self.rtte = RttEstimator::default();
         self.assembler = Assembler::new(self.rx_buffer.capacity());
         self.tx_buffer.clear();
@@ -6761,7 +6759,7 @@ mod test {
     #[test]
     fn test_timer_retransmit() {
         const RTO: Duration = Duration::from_millis(100);
-        let mut r = Timer::default();
+        let mut r = Timer::new();
         assert_eq!(r.should_retransmit(Instant::from_secs(1)), None);
         r.set_for_retransmit(Instant::from_millis(1000), RTO);
         assert_eq!(r.should_retransmit(Instant::from_millis(1000)), None);

utils/packet2pcap.rs

@@ -1,10 +1,9 @@
 use getopts::Options;
 use smoltcp::phy::{PcapLinkType, PcapSink};
 use smoltcp::time::Instant;
-use std::cell::RefCell;
 use std::env;
 use std::fs::File;
-use std::io::{self, Read, Write};
+use std::io::{self, Read};
 use std::path::Path;
 use std::process::exit;
 
@@ -17,12 +16,9 @@ fn convert(
     let mut packet = Vec::new();
     packet_file.read_to_end(&mut packet)?;
 
-    let pcap = RefCell::new(Vec::new());
-    PcapSink::global_header(&pcap, link_type);
-    PcapSink::packet(&pcap, Instant::from_millis(0), &packet[..]);
-
     let mut pcap_file = File::create(pcap_filename)?;
-    pcap_file.write_all(&pcap.borrow()[..])?;
+    PcapSink::global_header(&mut pcap_file, link_type);
+    PcapSink::packet(&mut pcap_file, Instant::from_millis(0), &packet[..]);
 
     Ok(())
 }