From 098baa282549efc25e80e0818ff1a24852542ac7 Mon Sep 17 00:00:00 2001
From: Teoh Han Hui <teohhanhui@gmail.com>
Date: Fri, 10 May 2024 23:27:14 +0800
Subject: [PATCH] Use std::os::unix::net::UnixStream::pair instead of
 rustix::net::socketpair

Assuming UnixStream in std is more widely used / battle tested, and has
more eyes on it.

Also, UnixStream is a higher-level wrapper, even though we are just
directly converting it into OwnedFd for now.

Signed-off-by: Teoh Han Hui <teohhanhui@gmail.com>
---
 crates/krun/Cargo.toml      |  2 +-
 crates/krun/src/bin/krun.rs |  2 +-
 crates/krun/src/net.rs      | 35 ++++++++++++++++-------------------
 3 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/crates/krun/Cargo.toml b/crates/krun/Cargo.toml
index 755f685..6ba9aef 100644
--- a/crates/krun/Cargo.toml
+++ b/crates/krun/Cargo.toml
@@ -15,7 +15,7 @@ env_logger = { workspace = true, features = ["auto-color", "humantime", "unstabl
 krun-sys = { workspace = true, features = [] }
 log = { workspace = true, features = ["kv"] }
 nix = { workspace = true, features = ["user"] }
-rustix = { workspace = true, features = ["net", "process", "std", "use-libc-auxv"] }
+rustix = { workspace = true, features = ["process", "std", "use-libc-auxv"] }
 utils = { workspace = true, features = [] }
 
 [features]
diff --git a/crates/krun/src/bin/krun.rs b/crates/krun/src/bin/krun.rs
index c3a5258..bb748f3 100644
--- a/crates/krun/src/bin/krun.rs
+++ b/crates/krun/src/bin/krun.rs
@@ -102,7 +102,7 @@ fn main() -> Result<()> {
                 .context("Failed to connect to `passt`")?
                 .into()
         } else {
-            start_passt().context("Failed to start `passt`")?
+            start_passt().context("Failed to start `passt`")?.into()
         };
         // SAFETY: `passt_fd` is an `OwnedFd` and consumed to prevent closing on drop.
         // See https://doc.rust-lang.org/std/io/index.html#io-safety
diff --git a/crates/krun/src/net.rs b/crates/krun/src/net.rs
index dff029a..c23a8dd 100644
--- a/crates/krun/src/net.rs
+++ b/crates/krun/src/net.rs
@@ -1,7 +1,7 @@
 use std::{
     fmt,
     os::{
-        fd::{AsRawFd, IntoRawFd, OwnedFd},
+        fd::{AsRawFd, IntoRawFd},
         unix::net::UnixStream,
     },
     path::Path,
@@ -10,10 +10,7 @@ use std::{
 
 use anyhow::{Context, Result};
 use log::debug;
-use rustix::{
-    io::dup,
-    net::{socketpair, AddressFamily, SocketFlags, SocketType},
-};
+use rustix::io::dup;
 
 #[derive(Copy, Clone, Eq, PartialEq, Debug)]
 pub enum NetMode {
@@ -28,23 +25,23 @@ where
     Ok(UnixStream::connect(passt_socket_path)?)
 }
 
-pub fn start_passt() -> Result<OwnedFd> {
-    let (parent_fd, child_fd) = socketpair(
-        AddressFamily::UNIX,
-        SocketType::STREAM,
-        // SAFETY: The child process should not inherit `parent_fd`.
-        SocketFlags::CLOEXEC,
-        None,
-    )?;
-
-    // SAFETY: The parent process should not keep `child_fd` open. It is an `OwnedFd` so it will be
-    // closed on drop.
+pub fn start_passt() -> Result<UnixStream> {
+    // SAFETY: The child process should not inherit the file descriptor of `parent_socket`.
+    // There is no documented guarantee of this, but the implementation as of writing atomically
+    // sets `SOCK_CLOEXEC`.
+    // See https://github.com/rust-lang/rust/blob/1.77.0/library/std/src/sys/pal/unix/net.rs#L124-L125
+    // See https://github.com/rust-lang/rust/issues/47946#issuecomment-364776373
+    let (parent_socket, child_socket) =
+        UnixStream::pair().context("Failed to create socket pair for `passt` child process")?;
+
+    // SAFETY: The parent process should not keep the file descriptor of `child_socket` open.
+    // It is a `UnixStream` so the file descriptor will be closed on drop.
     // See https://doc.rust-lang.org/std/io/index.html#io-safety
     //
     // The `dup` call clears the `FD_CLOEXEC` flag on the new `child_fd`, which should be inherited
     // by the child process.
-    let child_fd =
-        dup(child_fd).context("Failed to duplicate file descriptor for `passt` child process")?;
+    let child_fd = dup(child_socket)
+        .context("Failed to duplicate file descriptor for `passt` child process")?;
 
     debug!(fd = child_fd.as_raw_fd(); "passing fd to passt");
 
@@ -59,5 +56,5 @@ pub fn start_passt() -> Result<OwnedFd> {
         return Err(err).context("Failed to execute `passt` as child process");
     }
 
-    Ok(parent_fd)
+    Ok(parent_socket)
 }