Merge #1100

1100: net: remove unsafe, update smoltcp. r=Dirbaio a=Dirbaio

bors r+

Co-authored-by: Dario Nieuwenhuis <[email protected]>

5 files changed, 73 insertions, 99 deletions

diff --git a/embassy-net/Cargo.toml b/embassy-net/Cargo.toml
index 86d4aa105..ac338843d 100644
--- a/embassy-net/Cargo.toml
+++ b/embassy-net/Cargo.toml
@@ -57,7 +57,7 @@ embedded-nal-async = { version = "0.3.0", optional = true }
 [dependencies.smoltcp]
 version = "0.8.0"
 git = "https://github.com/smoltcp-rs/smoltcp"
-rev = "ed0cf16750a42f30e31fcaf5347915592924b1e3"
+rev = "b7a7c4b1c56e8d4c2524c1e3a056c745a13cc09f"
 default-features = false
 features = [
   "proto-ipv4",
diff --git a/embassy-net/src/device.rs b/embassy-net/src/device.rs
index c183bd58a..4bdfd7720 100644
--- a/embassy-net/src/device.rs
+++ b/embassy-net/src/device.rs
@@ -12,8 +12,6 @@ pub enum LinkState {
     Up,
 }
 
-// 'static required due to the "fake GAT" in smoltcp::phy::Device.
-// https://github.com/smoltcp-rs/smoltcp/pull/572
 pub trait Device {
     fn is_transmit_ready(&mut self) -> bool;
     fn transmit(&mut self, pkt: PacketBuf);
@@ -25,7 +23,7 @@ pub trait Device {
     fn ethernet_address(&self) -> [u8; 6];
 }
 
-impl<T: ?Sized + Device> Device for &'static mut T {
+impl<T: ?Sized + Device> Device for &mut T {
     fn is_transmit_ready(&mut self) -> bool {
         T::is_transmit_ready(self)
     }
@@ -63,11 +61,11 @@ impl<D: Device> DeviceAdapter<D> {
     }
 }
 
-impl<'a, D: Device + 'static> SmolDevice<'a> for DeviceAdapter<D> {
-    type RxToken = RxToken;
-    type TxToken = TxToken<'a, D>;
+impl<D: Device> SmolDevice for DeviceAdapter<D> {
+    type RxToken<'a> = RxToken where Self: 'a;
+    type TxToken<'a> = TxToken<'a, D> where Self: 'a;
 
-    fn receive(&'a mut self) -> Option<(Self::RxToken, Self::TxToken)> {
+    fn receive(&mut self) -> Option<(Self::RxToken<'_>, Self::TxToken<'_>)> {
         let tx_pkt = PacketBox::new(Packet::new())?;
         let rx_pkt = self.device.receive()?;
         let rx_token = RxToken { pkt: rx_pkt };
@@ -80,7 +78,7 @@ impl<'a, D: Device + 'static> SmolDevice<'a> for DeviceAdapter<D> {
     }
 
     /// Construct a transmit token.
-    fn transmit(&'a mut self) -> Option<Self::TxToken> {
+    fn transmit(&mut self) -> Option<Self::TxToken<'_>> {
         if !self.device.is_transmit_ready() {
             return None;
         }
diff --git a/embassy-net/src/stack.rs b/embassy-net/src/stack.rs
index 3a7610758..5c4fb0442 100644
--- a/embassy-net/src/stack.rs
+++ b/embassy-net/src/stack.rs
@@ -1,4 +1,4 @@
-use core::cell::UnsafeCell;
+use core::cell::RefCell;
 use core::future::{poll_fn, Future};
 use core::task::{Context, Poll};
 
@@ -62,8 +62,8 @@ pub enum ConfigStrategy {
 }
 
 pub struct Stack<D: Device> {
-    pub(crate) socket: UnsafeCell<SocketStack>,
-    inner: UnsafeCell<Inner<D>>,
+    pub(crate) socket: RefCell<SocketStack>,
+    inner: RefCell<Inner<D>>,
 }
 
 struct Inner<D: Device> {
@@ -81,8 +81,6 @@ pub(crate) struct SocketStack {
     next_local_port: u16,
 }
 
-unsafe impl<D: Device> Send for Stack<D> {}
-
 impl<D: Device + 'static> Stack<D> {
     pub fn new<const ADDR: usize, const SOCK: usize, const NEIGH: usize>(
         device: D,
@@ -143,40 +141,38 @@ impl<D: Device + 'static> Stack<D> {
         }
 
         Self {
-            socket: UnsafeCell::new(socket),
-            inner: UnsafeCell::new(inner),
+            socket: RefCell::new(socket),
+            inner: RefCell::new(inner),
         }
     }
 
-    /// SAFETY: must not call reentrantly.
-    unsafe fn with<R>(&self, f: impl FnOnce(&SocketStack, &Inner<D>) -> R) -> R {
-        f(&*self.socket.get(), &*self.inner.get())
+    fn with<R>(&self, f: impl FnOnce(&SocketStack, &Inner<D>) -> R) -> R {
+        f(&*self.socket.borrow(), &*self.inner.borrow())
     }
 
-    /// SAFETY: must not call reentrantly.
-    unsafe fn with_mut<R>(&self, f: impl FnOnce(&mut SocketStack, &mut Inner<D>) -> R) -> R {
-        f(&mut *self.socket.get(), &mut *self.inner.get())
+    fn with_mut<R>(&self, f: impl FnOnce(&mut SocketStack, &mut Inner<D>) -> R) -> R {
+        f(&mut *self.socket.borrow_mut(), &mut *self.inner.borrow_mut())
     }
 
     pub fn ethernet_address(&self) -> [u8; 6] {
-        unsafe { self.with(|_s, i| i.device.device.ethernet_address()) }
+        self.with(|_s, i| i.device.device.ethernet_address())
     }
 
     pub fn is_link_up(&self) -> bool {
-        unsafe { self.with(|_s, i| i.link_up) }
+        self.with(|_s, i| i.link_up)
     }
 
     pub fn is_config_up(&self) -> bool {
-        unsafe { self.with(|_s, i| i.config.is_some()) }
+        self.with(|_s, i| i.config.is_some())
     }
 
     pub fn config(&self) -> Option<Config> {
-        unsafe { self.with(|_s, i| i.config.clone()) }
+        self.with(|_s, i| i.config.clone())
     }
 
     pub async fn run(&self) -> ! {
         poll_fn(|cx| {
-            unsafe { self.with_mut(|s, i| i.poll(cx, s)) }
+            self.with_mut(|s, i| i.poll(cx, s));
             Poll::<()>::Pending
         })
         .await;
@@ -270,21 +266,12 @@ impl<D: Device + 'static> Inner<D> {
                     None => {}
                     Some(dhcpv4::Event::Deconfigured) => self.unapply_config(s),
                     Some(dhcpv4::Event::Configured(config)) => {
-                        let mut dns_servers = Vec::new();
-                        for s in &config.dns_servers {
-                            if let Some(addr) = s {
-                                dns_servers.push(addr.clone()).unwrap();
-                            }
-                        }
-
-                        self.apply_config(
-                            s,
-                            Config {
-                                address: config.address,
-                                gateway: config.router,
-                                dns_servers,
-                            },
-                        )
+                        let config = Config {
+                            address: config.address,
+                            gateway: config.router,
+                            dns_servers: config.dns_servers,
+                        };
+                        self.apply_config(s, config)
                     }
                 }
             } else if old_link_up {
diff --git a/embassy-net/src/tcp.rs b/embassy-net/src/tcp.rs
index 85d9e5ee1..73cf2d4e4 100644
--- a/embassy-net/src/tcp.rs
+++ b/embassy-net/src/tcp.rs
@@ -1,4 +1,4 @@
-use core::cell::UnsafeCell;
+use core::cell::RefCell;
 use core::future::poll_fn;
 use core::mem;
 use core::task::Poll;
@@ -68,8 +68,7 @@ impl<'a> TcpWriter<'a> {
 
 impl<'a> TcpSocket<'a> {
     pub fn new<D: Device>(stack: &'a Stack<D>, rx_buffer: &'a mut [u8], tx_buffer: &'a mut [u8]) -> Self {
-        // safety: not accessed reentrantly.
-        let s = unsafe { &mut *stack.socket.get() };
+        let s = &mut *stack.socket.borrow_mut();
         let rx_buffer: &'static mut [u8] = unsafe { mem::transmute(rx_buffer) };
         let tx_buffer: &'static mut [u8] = unsafe { mem::transmute(tx_buffer) };
         let handle = s.sockets.add(tcp::Socket::new(
@@ -93,17 +92,18 @@ impl<'a> TcpSocket<'a> {
     where
         T: Into<IpEndpoint>,
     {
-        // safety: not accessed reentrantly.
-        let local_port = unsafe { &mut *self.io.stack.get() }.get_local_port();
+        let local_port = self.io.stack.borrow_mut().get_local_port();
 
-        // safety: not accessed reentrantly.
-        match unsafe { self.io.with_mut(|s, i| s.connect(i, remote_endpoint, local_port)) } {
+        match {
+            self.io
+                .with_mut(|s, i| s.connect(i.context(), remote_endpoint, local_port))
+        } {
             Ok(()) => {}
             Err(tcp::ConnectError::InvalidState) => return Err(ConnectError::InvalidState),
             Err(tcp::ConnectError::Unaddressable) => return Err(ConnectError::NoRoute),
         }
 
-        poll_fn(|cx| unsafe {
+        poll_fn(|cx| {
             self.io.with_mut(|s, _| match s.state() {
                 tcp::State::Closed | tcp::State::TimeWait => Poll::Ready(Err(ConnectError::ConnectionReset)),
                 tcp::State::Listen => unreachable!(),
@@ -121,14 +121,13 @@ impl<'a> TcpSocket<'a> {
     where
         T: Into<IpListenEndpoint>,
     {
-        // safety: not accessed reentrantly.
-        match unsafe { self.io.with_mut(|s, _| s.listen(local_endpoint)) } {
+        match self.io.with_mut(|s, _| s.listen(local_endpoint)) {
             Ok(()) => {}
             Err(tcp::ListenError::InvalidState) => return Err(AcceptError::InvalidState),
             Err(tcp::ListenError::Unaddressable) => return Err(AcceptError::InvalidPort),
         }
 
-        poll_fn(|cx| unsafe {
+        poll_fn(|cx| {
             self.io.with_mut(|s, _| match s.state() {
                 tcp::State::Listen | tcp::State::SynSent | tcp::State::SynReceived => {
                     s.register_send_waker(cx.waker());
@@ -149,51 +148,49 @@ impl<'a> TcpSocket<'a> {
     }
 
     pub fn set_timeout(&mut self, duration: Option<Duration>) {
-        unsafe { self.io.with_mut(|s, _| s.set_timeout(duration)) }
+        self.io.with_mut(|s, _| s.set_timeout(duration))
     }
 
     pub fn set_keep_alive(&mut self, interval: Option<Duration>) {
-        unsafe { self.io.with_mut(|s, _| s.set_keep_alive(interval)) }
+        self.io.with_mut(|s, _| s.set_keep_alive(interval))
     }
 
     pub fn set_hop_limit(&mut self, hop_limit: Option<u8>) {
-        unsafe { self.io.with_mut(|s, _| s.set_hop_limit(hop_limit)) }
+        self.io.with_mut(|s, _| s.set_hop_limit(hop_limit))
     }
 
     pub fn local_endpoint(&self) -> Option<IpEndpoint> {
-        unsafe { self.io.with(|s, _| s.local_endpoint()) }
+        self.io.with(|s, _| s.local_endpoint())
     }
 
     pub fn remote_endpoint(&self) -> Option<IpEndpoint> {
-        unsafe { self.io.with(|s, _| s.remote_endpoint()) }
+        self.io.with(|s, _| s.remote_endpoint())
     }
 
     pub fn state(&self) -> tcp::State {
-        unsafe { self.io.with(|s, _| s.state()) }
+        self.io.with(|s, _| s.state())
     }
 
     pub fn close(&mut self) {
-        unsafe { self.io.with_mut(|s, _| s.close()) }
+        self.io.with_mut(|s, _| s.close())
     }
 
     pub fn abort(&mut self) {
-        unsafe { self.io.with_mut(|s, _| s.abort()) }
+        self.io.with_mut(|s, _| s.abort())
     }
 
     pub fn may_send(&self) -> bool {
-        unsafe { self.io.with(|s, _| s.may_send()) }
+        self.io.with(|s, _| s.may_send())
     }
 
     pub fn may_recv(&self) -> bool {
-        unsafe { self.io.with(|s, _| s.may_recv()) }
+        self.io.with(|s, _| s.may_recv())
     }
 }
 
 impl<'a> Drop for TcpSocket<'a> {
     fn drop(&mut self) {
-        // safety: not accessed reentrantly.
-        let s = unsafe { &mut *self.io.stack.get() };
-        s.sockets.remove(self.io.handle);
+        self.io.stack.borrow_mut().sockets.remove(self.io.handle);
     }
 }
 
@@ -201,21 +198,19 @@ impl<'a> Drop for TcpSocket<'a> {
 
 #[derive(Copy, Clone)]
 struct TcpIo<'a> {
-    stack: &'a UnsafeCell<SocketStack>,
+    stack: &'a RefCell<SocketStack>,
     handle: SocketHandle,
 }
 
 impl<'d> TcpIo<'d> {
-    /// SAFETY: must not call reentrantly.
-    unsafe fn with<R>(&self, f: impl FnOnce(&tcp::Socket, &Interface) -> R) -> R {
-        let s = &*self.stack.get();
+    fn with<R>(&self, f: impl FnOnce(&tcp::Socket, &Interface) -> R) -> R {
+        let s = &*self.stack.borrow();
         let socket = s.sockets.get::<tcp::Socket>(self.handle);
         f(socket, &s.iface)
     }
 
-    /// SAFETY: must not call reentrantly.
-    unsafe fn with_mut<R>(&mut self, f: impl FnOnce(&mut tcp::Socket, &mut Interface) -> R) -> R {
-        let s = &mut *self.stack.get();
+    fn with_mut<R>(&mut self, f: impl FnOnce(&mut tcp::Socket, &mut Interface) -> R) -> R {
+        let s = &mut *self.stack.borrow_mut();
         let socket = s.sockets.get_mut::<tcp::Socket>(self.handle);
         let res = f(socket, &mut s.iface);
         s.waker.wake();
@@ -223,7 +218,7 @@ impl<'d> TcpIo<'d> {
     }
 
     async fn read(&mut self, buf: &mut [u8]) -> Result<usize, Error> {
-        poll_fn(move |cx| unsafe {
+        poll_fn(move |cx| {
             // CAUTION: smoltcp semantics around EOF are different to what you'd expect
             // from posix-like IO, so we have to tweak things here.
             self.with_mut(|s, _| match s.recv_slice(buf) {
@@ -244,7 +239,7 @@ impl<'d> TcpIo<'d> {
     }
 
     async fn write(&mut self, buf: &[u8]) -> Result<usize, Error> {
-        poll_fn(move |cx| unsafe {
+        poll_fn(move |cx| {
             self.with_mut(|s, _| match s.send_slice(buf) {
                 // Not ready to send (no space in the tx buffer)
                 Ok(0) => {
@@ -332,6 +327,7 @@ mod embedded_io_impls {
 
 #[cfg(all(feature = "unstable-traits", feature = "nightly"))]
 pub mod client {
+    use core::cell::UnsafeCell;
     use core::mem::MaybeUninit;
     use core::ptr::NonNull;
 
diff --git a/embassy-net/src/udp.rs b/embassy-net/src/udp.rs
index f2e33493c..4ddad77d4 100644
--- a/embassy-net/src/udp.rs
+++ b/embassy-net/src/udp.rs
@@ -1,4 +1,4 @@
-use core::cell::UnsafeCell;
+use core::cell::RefCell;
 use core::future::poll_fn;
 use core::mem;
 use core::task::Poll;
@@ -27,7 +27,7 @@ pub enum Error {
 }
 
 pub struct UdpSocket<'a> {
-    stack: &'a UnsafeCell<SocketStack>,
+    stack: &'a RefCell<SocketStack>,
     handle: SocketHandle,
 }
 
@@ -39,8 +39,7 @@ impl<'a> UdpSocket<'a> {
         tx_meta: &'a mut [PacketMetadata],
         tx_buffer: &'a mut [u8],
     ) -> Self {
-        // safety: not accessed reentrantly.
-        let s = unsafe { &mut *stack.socket.get() };
+        let s = &mut *stack.socket.borrow_mut();
 
         let rx_meta: &'static mut [PacketMetadata] = unsafe { mem::transmute(rx_meta) };
         let rx_buffer: &'static mut [u8] = unsafe { mem::transmute(rx_buffer) };
@@ -63,30 +62,26 @@ impl<'a> UdpSocket<'a> {
     {
         let mut endpoint = endpoint.into();
 
-        // safety: not accessed reentrantly.
         if endpoint.port == 0 {
             // If user didn't specify port allocate a dynamic port.
-            endpoint.port = unsafe { &mut *self.stack.get() }.get_local_port();
+            endpoint.port = self.stack.borrow_mut().get_local_port();
         }
 
-        // safety: not accessed reentrantly.
-        match unsafe { self.with_mut(|s, _| s.bind(endpoint)) } {
+        match self.with_mut(|s, _| s.bind(endpoint)) {
             Ok(()) => Ok(()),
             Err(udp::BindError::InvalidState) => Err(BindError::InvalidState),
             Err(udp::BindError::Unaddressable) => Err(BindError::NoRoute),
         }
     }
 
-    /// SAFETY: must not call reentrantly.
-    unsafe fn with<R>(&self, f: impl FnOnce(&udp::Socket, &Interface) -> R) -> R {
-        let s = &*self.stack.get();
+    fn with<R>(&self, f: impl FnOnce(&udp::Socket, &Interface) -> R) -> R {
+        let s = &*self.stack.borrow();
         let socket = s.sockets.get::<udp::Socket>(self.handle);
         f(socket, &s.iface)
     }
 
-    /// SAFETY: must not call reentrantly.
-    unsafe fn with_mut<R>(&self, f: impl FnOnce(&mut udp::Socket, &mut Interface) -> R) -> R {
-        let s = &mut *self.stack.get();
+    fn with_mut<R>(&self, f: impl FnOnce(&mut udp::Socket, &mut Interface) -> R) -> R {
+        let s = &mut *self.stack.borrow_mut();
         let socket = s.sockets.get_mut::<udp::Socket>(self.handle);
         let res = f(socket, &mut s.iface);
         s.waker.wake();
@@ -94,7 +89,7 @@ impl<'a> UdpSocket<'a> {
     }
 
     pub async fn recv_from(&self, buf: &mut [u8]) -> Result<(usize, IpEndpoint), Error> {
-        poll_fn(move |cx| unsafe {
+        poll_fn(move |cx| {
             self.with_mut(|s, _| match s.recv_slice(buf) {
                 Ok(x) => Poll::Ready(Ok(x)),
                 // No data ready
@@ -113,7 +108,7 @@ impl<'a> UdpSocket<'a> {
         T: Into<IpEndpoint>,
     {
         let remote_endpoint = remote_endpoint.into();
-        poll_fn(move |cx| unsafe {
+        poll_fn(move |cx| {
             self.with_mut(|s, _| match s.send_slice(buf, remote_endpoint) {
                 // Entire datagram has been sent
                 Ok(()) => Poll::Ready(Ok(())),
@@ -128,30 +123,28 @@ impl<'a> UdpSocket<'a> {
     }
 
     pub fn endpoint(&self) -> IpListenEndpoint {
-        unsafe { self.with(|s, _| s.endpoint()) }
+        self.with(|s, _| s.endpoint())
     }
 
     pub fn is_open(&self) -> bool {
-        unsafe { self.with(|s, _| s.is_open()) }
+        self.with(|s, _| s.is_open())
     }
 
     pub fn close(&mut self) {
-        unsafe { self.with_mut(|s, _| s.close()) }
+        self.with_mut(|s, _| s.close())
     }
 
     pub fn may_send(&self) -> bool {
-        unsafe { self.with(|s, _| s.can_send()) }
+        self.with(|s, _| s.can_send())
     }
 
     pub fn may_recv(&self) -> bool {
-        unsafe { self.with(|s, _| s.can_recv()) }
+        self.with(|s, _| s.can_recv())
     }
 }
 
 impl Drop for UdpSocket<'_> {
     fn drop(&mut self) {
-        // safety: not accessed reentrantly.
-        let s = unsafe { &mut *self.stack.get() };
-        s.sockets.remove(self.handle);
+        self.stack.borrow_mut().sockets.remove(self.handle);
     }
 }