[SR-11354] Foundation.Process inherits file descriptors into the child process

[weissi]

Previous ID	SR-11354
Radar	rdar://problem/54600455
Original Reporter	@weissi
Type	Bug
Status	Resolved
Resolution	Done

Attachment: Download

Additional Detail from JIRA

Votes	1
Component/s	Foundation
Labels	Bug
Assignee	None
Priority	Medium

md5: 6bb31ba0bafc6fc1de60d481ac760a2d

Issue Description:

The following program when compiled and run like swiftc test.swift && ./test should print

ok1
the line above should read 'ok1'

which is does on Darwin. The reason is that Foundation.Process is supposed to close the parents file descriptors so that they're not inherited into the child.

However, on Linux, this is not the case:

$ docker run -it --rm -v "$PWD:$PWD" -w "$PWD" swift:5.0.2 bash -c 'swiftc test.swift && ./test'
ok1
THIS IS BAD
the line above should read 'ok1'

#if os(Linux)
import Glibc
#else
import Darwin
#endif
import Foundation


if let stdout2 = CommandLine.arguments.dropFirst().first.flatMap(CInt.init) {
    write(stdout2, "THIS IS BAD\n", 12)
    print("the line above should read 'ok1'")
} else {
    let stdout2 = dup(1)
    write(stdout2, "ok1\n", 4)
    let p = Process()
    p.executableURL = URL(fileURLWithPath: CommandLine.arguments.first!)
    p.arguments = [ "\(stdout2)" ]
    try! p.run()
    p.waitUntilExit()
}

[weissi]

@swift-ci create

[millenomi]

That's because Process on Darwin uses the non-portable POSIX_SPAWN_CLOEXEC_DEFAULT flag to have the kernel do it, which we didn't port over.

Apparently, on Linux, we can implement it via the approach in point 2 of https://docs.fedoraproject.org/en-US/Fedora_Security_Team/1/html/Defensive_Coding/sect-Defensive_Coding-Tasks-Descriptors-Child_Processes.html.

[millenomi]

@compnerd gwenm (JIRA User) the above approach has a POSIX portable approximation (add file close ops for all fds from 0 to FD_SETSIZE - 1), but Windows has no fds; is Windows affected similarly? (IIRC no?)

[weissi]

@millenomi the only thing I think will work is to set POSIX_SPAWN_CLOEXEC_DEFAULT on Darwin and do

        for fd in 3..<getdtablesize() {
            posix(_CFPosixSpawnFileActionsAddClose(fileActions, fd))
        }

on Linux. Closing up to getdtablesize() is the standard thing to do between fork and execve.

[millenomi]

The links are outdated, then. Sigh, Internet.

[weissi]

@millenomi something (needs a lot of cleanup) like this: #2485

[swift-ci]

Comment by Gwen Mittertreiner (JIRA)

I checked this on Windows (with some modification: write on Windows asserts by default if given a bad fd), it works as excepted.

import Foundation
import MSVCRT
import WinSDK
func dontFailPlease(e: UnsafePointer<UInt16>?, f: UnsafePointer<UInt16>?, file: UnsafePointer<UInt16>?, line: UInt32, pReserved: UInt) { }

if let stdout2 = CommandLine.arguments.dropFirst().first.flatMap(CInt.init) {
  _set_invalid_parameter_handler(dontFailPlease)
  write(stdout2, "THIS IS BAD\n", 12)
  print("the line above should read 'ok1'")
} else {
  let stdout2 = dup(1)
  write(stdout2, "ok1\n", 4)
  let p = Process()
  p.executableURL = URL(fileURLWithPath: CommandLine.arguments.first!)
  p.arguments = [ "\(stdout2)" ]
  try! p.run()
  p.waitUntilExit()
}

Running it:

C:\Users\gwenm\tmp\fds ▶ ./main.exe
ok1
the line above should read 'ok1'

[swift-ci]

Comment by Gregor Milos (Grzegorz Miłoś) (JIRA)

@weissi this should be closed now that the PR is merged? #2485

[weissi]

Yes, thanks Gregor!