rust io safety: convert RawFd to BorrowedFd<>

This is much safer because we no longer pass a single int around. With
BorrowedFd<> the compiler will actually make sure the underlying file
will not be closed before we use it.

I could have made every struct generic to accept the AsFd trait but
that would have resulted in IMO more complex code without benefit.

Fixes #806

Signed-off-by: Paul Holzinger <pholzing@redhat.com>

8 files changed, 45 insertions, 49 deletions

diff --git a/examples/host-device-plugin.rs b/examples/host-device-plugin.rs
index d214520..a45bc88 100644
--- a/examples/host-device-plugin.rs
+++ b/examples/host-device-plugin.rs
@@ -3,6 +3,7 @@
 use std::{
     collections::HashMap,
     net::{Ipv4Addr, Ipv6Addr},
+    os::fd::AsFd,
 };
 
 use netavark::{
@@ -80,7 +81,8 @@ impl Plugin for Exec {
             }
         }
 
-        host.netlink.set_link_ns(link.header.index, netns.fd)?;
+        host.netlink
+            .set_link_ns(link.header.index, netns.file.as_fd())?;
 
         // interfaces map, but we only ever expect one, for response
         let mut interfaces: HashMap<String, types::NetInterface> = HashMap::new();
@@ -113,7 +115,9 @@ impl Plugin for Exec {
 
         let link = netns.netlink.get_link(netlink::LinkID::Name(name))?;
 
-        netns.netlink.set_link_ns(link.header.index, host.fd)?;
+        netns
+            .netlink
+            .set_link_ns(link.header.index, host.file.as_fd())?;
 
         Ok(())
     }
diff --git a/src/commands/setup.rs b/src/commands/setup.rs
index d9154d8..b7c1099 100644
--- a/src/commands/setup.rs
+++ b/src/commands/setup.rs
@@ -12,6 +12,7 @@ use clap::Parser;
 use log::{debug, error, info};
 use std::collections::HashMap;
 use std::fs::{self};
+use std::os::fd::AsFd;
 use std::path::Path;
 
 #[derive(Parser, Debug)]
@@ -75,8 +76,8 @@ impl Setup {
                     container_id: &network_options.container_id,
                     container_name: &network_options.container_name,
                     container_dns_servers: &network_options.dns_servers,
-                    netns_host: hostns.fd,
-                    netns_container: netns.fd,
+                    netns_host: hostns.file.as_fd(),
+                    netns_container: netns.file.as_fd(),
                     netns_path: &self.network_namespace_path,
                     network,
                     per_network_opts,
diff --git a/src/commands/teardown.rs b/src/commands/teardown.rs
index 1fb947b..263bc64 100644
--- a/src/commands/teardown.rs
+++ b/src/commands/teardown.rs
@@ -8,6 +8,7 @@ use crate::{firewall, network};
 use clap::builder::NonEmptyStringValueParser;
 use clap::Parser;
 use log::debug;
+use std::os::fd::AsFd;
 use std::path::Path;
 
 #[derive(Parser, Debug)]
@@ -105,8 +106,8 @@ impl Teardown {
                     container_id: &network_options.container_id,
                     container_name: &network_options.container_name,
                     container_dns_servers: &network_options.dns_servers,
-                    netns_host: hostns.fd,
-                    netns_container: netns.fd,
+                    netns_host: hostns.file.as_fd(),
+                    netns_container: netns.file.as_fd(),
                     netns_path: &self.network_namespace_path,
                     network,
                     per_network_opts,
diff --git a/src/network/bridge.rs b/src/network/bridge.rs
index e87134f..625950f 100644
--- a/src/network/bridge.rs
+++ b/src/network/bridge.rs
@@ -1,4 +1,4 @@
-use std::{collections::HashMap, net::IpAddr, os::unix::prelude::RawFd, sync::Once};
+use std::{collections::HashMap, net::IpAddr, os::fd::BorrowedFd, sync::Once};
 
 use ipnet::IpNet;
 use log::{debug, error};
@@ -475,8 +475,8 @@ fn create_interfaces(
     netns: &mut netlink::Socket,
     data: &InternalData,
     internal: bool,
-    hostns_fd: RawFd,
-    netns_fd: RawFd,
+    hostns_fd: BorrowedFd<'_>,
+    netns_fd: BorrowedFd<'_>,
 ) -> NetavarkResult<String> {
     let bridge = match host.get_link(netlink::LinkID::Name(
         data.bridge_interface_name.to_string(),
@@ -540,20 +540,20 @@ fn create_interfaces(
 }
 
 /// return the container veth mac address
-fn create_veth_pair(
+fn create_veth_pair<'fd>(
     host: &mut netlink::Socket,
     netns: &mut netlink::Socket,
     data: &InternalData,
     primary_index: u32,
     internal: bool,
-    hostns_fd: RawFd,
-    netns_fd: RawFd,
+    hostns_fd: BorrowedFd<'fd>,
+    netns_fd: BorrowedFd<'fd>,
 ) -> NetavarkResult<String> {
     let mut peer_opts =
         netlink::CreateLinkOptions::new(data.container_interface_name.to_string(), InfoKind::Veth);
     peer_opts.mac = data.mac_address.clone().unwrap_or_default();
     peer_opts.mtu = data.mtu;
-    peer_opts.netns = netns_fd;
+    peer_opts.netns = Some(netns_fd);
 
     let mut peer = LinkMessage::default();
     netlink::parse_create_link_options(&mut peer, peer_opts);
diff --git a/src/network/core_utils.rs b/src/network/core_utils.rs
index 36a4c17..0b06780 100644
--- a/src/network/core_utils.rs
+++ b/src/network/core_utils.rs
@@ -282,11 +282,8 @@ impl CoreUtils {
     }
 }
 
-pub fn join_netns(fd: RawFd) -> NetavarkResult<()> {
-    match sched::setns(
-        unsafe { BorrowedFd::borrow_raw(fd) },
-        sched::CloneFlags::CLONE_NEWNET,
-    ) {
+pub fn join_netns<Fd: AsFd>(fd: Fd) -> NetavarkResult<()> {
+    match sched::setns(fd, sched::CloneFlags::CLONE_NEWNET) {
         Ok(_) => Ok(()),
         Err(e) => Err(NetavarkError::wrap(
             "setns",
@@ -312,7 +309,6 @@ pub struct NamespaceOptions {
     /// Note we have to return the File object since the fd is only valid
     /// as long as the File object is valid
     pub file: File,
-    pub fd: RawFd,
     pub netlink: netlink::Socket,
 }
 
@@ -323,10 +319,9 @@ pub fn open_netlink_sockets(
     let hostns = open_netlink_socket("/proc/self/ns/net").wrap("open host netns")?;
 
     let host_socket = netlink::Socket::new().wrap("host netlink socket")?;
-
     exec_netns!(
-        hostns.1,
-        netns.1,
+        hostns.as_fd(),
+        netns.as_fd(),
         res,
         netlink::Socket::new().wrap("netns netlink socket")
     );
@@ -334,22 +329,18 @@ pub fn open_netlink_sockets(
     let netns_sock = res?;
     Ok((
         NamespaceOptions {
-            file: hostns.0,
-            fd: hostns.1,
+            file: hostns,
             netlink: host_socket,
         },
         NamespaceOptions {
-            file: netns.0,
-            fd: netns.1,
+            file: netns,
             netlink: netns_sock,
         },
     ))
 }
 
-fn open_netlink_socket(netns_path: &str) -> NetavarkResult<(File, RawFd)> {
-    let ns = wrap!(File::open(netns_path), format!("open {netns_path}"))?;
-    let ns_fd = ns.as_raw_fd();
-    Ok((ns, ns_fd))
+fn open_netlink_socket(netns_path: &str) -> NetavarkResult<File> {
+    wrap!(File::open(netns_path), format!("open {netns_path}"))
 }
 
 pub fn add_default_routes(
diff --git a/src/network/driver.rs b/src/network/driver.rs
index c2b7d6e..787b468 100644
--- a/src/network/driver.rs
+++ b/src/network/driver.rs
@@ -4,7 +4,7 @@ use crate::{
     firewall::FirewallDriver,
 };
 
-use std::{net::IpAddr, path::Path};
+use std::{net::IpAddr, os::fd::BorrowedFd, path::Path};
 
 use super::{
     bridge::Bridge,
@@ -14,15 +14,14 @@ use super::{
     vlan::Vlan,
 };
 use std::os::unix::fs::PermissionsExt;
-use std::os::unix::io::RawFd;
 
 pub struct DriverInfo<'a> {
     pub firewall: &'a dyn FirewallDriver,
     pub container_id: &'a String,
     pub container_name: &'a String,
     pub container_dns_servers: &'a Option<Vec<IpAddr>>,
-    pub netns_host: RawFd,
-    pub netns_container: RawFd,
+    pub netns_host: BorrowedFd<'a>,
+    pub netns_container: BorrowedFd<'a>,
     pub netns_path: &'a str,
     pub network: &'a Network,
     pub per_network_opts: &'a PerNetworkOptions,
diff --git a/src/network/netlink.rs b/src/network/netlink.rs
index dd8f012..2e563b5 100644
--- a/src/network/netlink.rs
+++ b/src/network/netlink.rs
@@ -1,6 +1,6 @@
 use std::{
     net::{Ipv4Addr, Ipv6Addr},
-    os::unix::prelude::RawFd,
+    os::fd::{AsFd, AsRawFd, BorrowedFd},
 };
 
 use crate::{
@@ -28,7 +28,7 @@ pub struct Socket {
 }
 
 #[derive(Clone)]
-pub struct CreateLinkOptions {
+pub struct CreateLinkOptions<'fd> {
     pub name: String,
     kind: InfoKind,
     pub info_data: Option<InfoData>,
@@ -36,7 +36,7 @@ pub struct CreateLinkOptions {
     pub primary_index: u32,
     pub link: u32,
     pub mac: Vec<u8>,
-    pub netns: RawFd,
+    pub netns: Option<BorrowedFd<'fd>>,
 }
 
 pub enum LinkID {
@@ -174,10 +174,10 @@ impl Socket {
         Ok(())
     }
 
-    pub fn set_link_ns(&mut self, link_id: u32, netns_fd: i32) -> NetavarkResult<()> {
+    pub fn set_link_ns<Fd: AsFd>(&mut self, link_id: u32, netns: Fd) -> NetavarkResult<()> {
         let mut msg = LinkMessage::default();
         msg.header.index = link_id;
-        msg.nlas.push(Nla::NetNsFd(netns_fd));
+        msg.nlas.push(Nla::NetNsFd(netns.as_fd().as_raw_fd()));
 
         let result = self.make_netlink_request(RtnlMessage::SetLink(msg), NLM_F_ACK)?;
         expect_netlink_result!(result, 0);
@@ -484,7 +484,7 @@ impl Socket {
     }
 }
 
-impl CreateLinkOptions {
+impl CreateLinkOptions<'_> {
     pub fn new(name: String, kind: InfoKind) -> Self {
         CreateLinkOptions {
             name,
@@ -494,8 +494,7 @@ impl CreateLinkOptions {
             primary_index: 0,
             link: 0,
             mac: vec![],
-            // 0 is a valid fd, so use -1 by default
-            netns: -1,
+            netns: None,
         }
     }
 }
@@ -534,7 +533,7 @@ pub fn parse_create_link_options(msg: &mut LinkMessage, options: CreateLinkOptio
     }
 
     // add netnsfd
-    if options.netns > -1 {
-        msg.nlas.push(Nla::NetNsFd(options.netns));
+    if let Some(netns) = options.netns {
+        msg.nlas.push(Nla::NetNsFd(netns.as_raw_fd()));
     }
 }
diff --git a/src/network/vlan.rs b/src/network/vlan.rs
index eb055dc..95035eb 100644
--- a/src/network/vlan.rs
+++ b/src/network/vlan.rs
@@ -1,5 +1,6 @@
 use log::{debug, error};
-use std::{collections::HashMap, net::IpAddr, os::unix::prelude::RawFd};
+use std::os::fd::BorrowedFd;
+use std::{collections::HashMap, net::IpAddr};
 
 use netlink_packet_route::nlas::link::{InfoData, InfoIpVlan, InfoKind, InfoMacVlan, Nla};
 use rand::distributions::{Alphanumeric, DistString};
@@ -250,8 +251,8 @@ fn setup(
     netns: &mut netlink::Socket,
     if_name: &str,
     data: &InternalData,
-    hostns_fd: RawFd,
-    netns_fd: RawFd,
+    hostns_fd: BorrowedFd<'_>,
+    netns_fd: BorrowedFd<'_>,
     kind_data: &KindData,
 ) -> NetavarkResult<String> {
     let primary_ifname = match data.host_interface_name.as_ref() {
@@ -265,7 +266,7 @@ fn setup(
         KindData::IpVlan { mode } => {
             let mut opts = CreateLinkOptions::new(if_name.to_string(), InfoKind::IpVlan);
             opts.mtu = data.mtu;
-            opts.netns = netns_fd;
+            opts.netns = Some(netns_fd);
             opts.link = link.header.index;
             opts.info_data = Some(InfoData::IpVlan(vec![InfoIpVlan::Mode(*mode)]));
             opts
@@ -278,7 +279,7 @@ fn setup(
             let mut opts = CreateLinkOptions::new(if_name.to_string(), InfoKind::MacVlan);
             opts.mac = mac_address.clone().unwrap_or_default();
             opts.mtu = data.mtu;
-            opts.netns = netns_fd;
+            opts.netns = Some(netns_fd);
             opts.link = link.header.index;
 
             let mut mv_opts = vec![InfoMacVlan::Mode(*mode)];