Skip to content

Commit df2425e

Browse files
authored
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.
1 parent 48e2f41 commit df2425e

File tree

3 files changed

+17
-12
lines changed

3 files changed

+17
-12
lines changed

vminitd/Sources/vmexec/ExecCommand.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@ struct ExecCommand: ParsableCommand {
101101

102102
let data = Data(bytes: &masterFD, count: MemoryLayout.size(ofValue: masterFD))
103103
try syncPipe.write(contentsOf: data)
104-
try syncPipe.close()
105104

106105
// Wait for the grandparent to tell us that they acked our console.
107106
guard let data = try ackPipe.read(upToCount: App.ackConsole.count) else {

vminitd/Sources/vmexec/RunCommand.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ struct RunCommand: ParsableCommand {
9191

9292
let data = Data(bytes: &masterFD, count: MemoryLayout.size(ofValue: masterFD))
9393
try syncPipe.write(contentsOf: data)
94-
try syncPipe.close()
9594

9695
// Wait for the grandparent to tell us that they acked our console.
9796
guard let data = try ackPipe.read(upToCount: App.ackConsole.count) else {

vminitd/Sources/vminitd/ManagedProcess.swift

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ final class ManagedProcess: Sendable {
3030
private let process: Command
3131
private let state: Mutex<State>
3232
private let owningPid: Int32?
33-
private let ackPipe: FileHandle
34-
private let syncPipe: FileHandle
33+
private let ackPipe: Pipe
34+
private let syncPipe: Pipe
3535
private let terminal: Bool
3636
private let bundle: ContainerizationOCI.Bundle
3737
private let cgroupManager: Cgroup2Manager?
@@ -87,11 +87,11 @@ final class ManagedProcess: Sendable {
8787

8888
let syncPipe = Pipe()
8989
try syncPipe.setCloexec()
90-
self.syncPipe = syncPipe.fileHandleForReading
90+
self.syncPipe = syncPipe
9191

9292
let ackPipe = Pipe()
9393
try ackPipe.setCloexec()
94-
self.ackPipe = ackPipe.fileHandleForWriting
94+
self.ackPipe = ackPipe
9595

9696
let args: [String]
9797
if let owningPid {
@@ -157,15 +157,19 @@ extension ManagedProcess {
157157
// Start the underlying process.
158158
try process.start()
159159
defer {
160-
try? self.ackPipe.close()
161-
try? self.syncPipe.close()
160+
try? self.ackPipe.fileHandleForWriting.close()
161+
try? self.syncPipe.fileHandleForReading.close()
162+
try? self.ackPipe.fileHandleForReading.close()
163+
try? self.syncPipe.fileHandleForWriting.close()
162164
}
163165

164166
// Close our side of any pipes.
165167
try $0.io.closeAfterExec()
168+
try self.ackPipe.fileHandleForReading.close()
169+
try self.syncPipe.fileHandleForWriting.close()
166170

167171
let size = MemoryLayout<Int32>.size
168-
guard let piddata = try syncPipe.read(upToCount: size) else {
172+
guard let piddata = try syncPipe.fileHandleForReading.read(upToCount: size) else {
169173
throw ContainerizationError(.internalError, message: "no pid data from sync pipe")
170174
}
171175

@@ -201,7 +205,7 @@ extension ManagedProcess {
201205
metadata: [
202206
"pid": "\(pid)"
203207
])
204-
try self.ackPipe.write(contentsOf: Self.ackPid.data(using: .utf8)!)
208+
try self.ackPipe.fileHandleForWriting.write(contentsOf: Self.ackPid.data(using: .utf8)!)
205209

206210
if self.terminal {
207211
log.info(
@@ -211,7 +215,7 @@ extension ManagedProcess {
211215
])
212216

213217
// Wait for a new write that will contain the pty fd if we asked for one.
214-
guard let ptyFd = try syncPipe.read(upToCount: size) else {
218+
guard let ptyFd = try self.syncPipe.fileHandleForReading.read(upToCount: size) else {
215219
throw ContainerizationError(
216220
.internalError,
217221
message: "no pty data from sync pipe"
@@ -227,9 +231,12 @@ extension ManagedProcess {
227231
])
228232

229233
try $0.io.attach(pid: pid, fd: fd)
230-
try self.ackPipe.write(contentsOf: Self.ackConsole.data(using: .utf8)!)
234+
try self.ackPipe.fileHandleForWriting.write(contentsOf: Self.ackConsole.data(using: .utf8)!)
231235
}
232236

237+
// Wait for the syncPipe to close (after exec).
238+
_ = try self.syncPipe.fileHandleForReading.readToEnd()
239+
233240
log.info(
234241
"started managed process",
235242
metadata: [

0 commit comments

Comments
 (0)