From df2425ef7f53f29cc63b9cbcf1aecf6882e6fd7c Mon Sep 17 00:00:00 2001
From: Danny Canter <danny_canter@apple.com>
Date: Tue, 26 Aug 2025 09:34:13 -0700
Subject: [PATCH] vminitd: Wait for execvpe to return to continue (#275)

Today, because we don't wait for execvpe to finish to continue onwards,
it's possible that if you did an exec quick enough after starting an
init process for a container, that you could join the init processes
namespaces before pivot_root has taken place which is quite fun. Let's
wait for exec to finish (or an error to occur) to prevent this.
---
 vminitd/Sources/vmexec/ExecCommand.swift     |  1 -
 vminitd/Sources/vmexec/RunCommand.swift      |  1 -
 vminitd/Sources/vminitd/ManagedProcess.swift | 27 ++++++++++++--------
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/vminitd/Sources/vmexec/ExecCommand.swift b/vminitd/Sources/vmexec/ExecCommand.swift
index 501e118c..ffb9c7d7 100644
--- a/vminitd/Sources/vmexec/ExecCommand.swift
+++ b/vminitd/Sources/vmexec/ExecCommand.swift
@@ -101,7 +101,6 @@ struct ExecCommand: ParsableCommand {
 
                 let data = Data(bytes: &masterFD, count: MemoryLayout.size(ofValue: masterFD))
                 try syncPipe.write(contentsOf: data)
-                try syncPipe.close()
 
                 // Wait for the grandparent to tell us that they acked our console.
                 guard let data = try ackPipe.read(upToCount: App.ackConsole.count) else {
diff --git a/vminitd/Sources/vmexec/RunCommand.swift b/vminitd/Sources/vmexec/RunCommand.swift
index 73913c08..fe865289 100644
--- a/vminitd/Sources/vmexec/RunCommand.swift
+++ b/vminitd/Sources/vmexec/RunCommand.swift
@@ -91,7 +91,6 @@ struct RunCommand: ParsableCommand {
 
             let data = Data(bytes: &masterFD, count: MemoryLayout.size(ofValue: masterFD))
             try syncPipe.write(contentsOf: data)
-            try syncPipe.close()
 
             // Wait for the grandparent to tell us that they acked our console.
             guard let data = try ackPipe.read(upToCount: App.ackConsole.count) else {
diff --git a/vminitd/Sources/vminitd/ManagedProcess.swift b/vminitd/Sources/vminitd/ManagedProcess.swift
index b9200cf2..9e22684c 100644
--- a/vminitd/Sources/vminitd/ManagedProcess.swift
+++ b/vminitd/Sources/vminitd/ManagedProcess.swift
@@ -30,8 +30,8 @@ final class ManagedProcess: Sendable {
     private let process: Command
     private let state: Mutex<State>
     private let owningPid: Int32?
-    private let ackPipe: FileHandle
-    private let syncPipe: FileHandle
+    private let ackPipe: Pipe
+    private let syncPipe: Pipe
     private let terminal: Bool
     private let bundle: ContainerizationOCI.Bundle
     private let cgroupManager: Cgroup2Manager?
@@ -87,11 +87,11 @@ final class ManagedProcess: Sendable {
 
         let syncPipe = Pipe()
         try syncPipe.setCloexec()
-        self.syncPipe = syncPipe.fileHandleForReading
+        self.syncPipe = syncPipe
 
         let ackPipe = Pipe()
         try ackPipe.setCloexec()
-        self.ackPipe = ackPipe.fileHandleForWriting
+        self.ackPipe = ackPipe
 
         let args: [String]
         if let owningPid {
@@ -157,15 +157,19 @@ extension ManagedProcess {
             // Start the underlying process.
             try process.start()
             defer {
-                try? self.ackPipe.close()
-                try? self.syncPipe.close()
+                try? self.ackPipe.fileHandleForWriting.close()
+                try? self.syncPipe.fileHandleForReading.close()
+                try? self.ackPipe.fileHandleForReading.close()
+                try? self.syncPipe.fileHandleForWriting.close()
             }
 
             // Close our side of any pipes.
             try $0.io.closeAfterExec()
+            try self.ackPipe.fileHandleForReading.close()
+            try self.syncPipe.fileHandleForWriting.close()
 
             let size = MemoryLayout<Int32>.size
-            guard let piddata = try syncPipe.read(upToCount: size) else {
+            guard let piddata = try syncPipe.fileHandleForReading.read(upToCount: size) else {
                 throw ContainerizationError(.internalError, message: "no pid data from sync pipe")
             }
 
@@ -201,7 +205,7 @@ extension ManagedProcess {
                 metadata: [
                     "pid": "\(pid)"
                 ])
-            try self.ackPipe.write(contentsOf: Self.ackPid.data(using: .utf8)!)
+            try self.ackPipe.fileHandleForWriting.write(contentsOf: Self.ackPid.data(using: .utf8)!)
 
             if self.terminal {
                 log.info(
@@ -211,7 +215,7 @@ extension ManagedProcess {
                     ])
 
                 // Wait for a new write that will contain the pty fd if we asked for one.
-                guard let ptyFd = try syncPipe.read(upToCount: size) else {
+                guard let ptyFd = try self.syncPipe.fileHandleForReading.read(upToCount: size) else {
                     throw ContainerizationError(
                         .internalError,
                         message: "no pty data from sync pipe"
@@ -227,9 +231,12 @@ extension ManagedProcess {
                     ])
 
                 try $0.io.attach(pid: pid, fd: fd)
-                try self.ackPipe.write(contentsOf: Self.ackConsole.data(using: .utf8)!)
+                try self.ackPipe.fileHandleForWriting.write(contentsOf: Self.ackConsole.data(using: .utf8)!)
             }
 
+            // Wait for the syncPipe to close (after exec).
+            _ = try self.syncPipe.fileHandleForReading.readToEnd()
+
             log.info(
                 "started managed process",
                 metadata: [