Merge pull request opencontainers#4175 from cyphar/fd-file-switch

init: use *os.File for passed file descriptors

libcontainer/init_linux.go

@@ -141,24 +141,26 @@ func startInitialization() (retErr error) {
 		logrus.SetLevel(logrus.Level(logLevel))
 	}
 
-	logFD, err := strconv.Atoi(os.Getenv("_LIBCONTAINER_LOGPIPE"))
+	logFd, err := strconv.Atoi(os.Getenv("_LIBCONTAINER_LOGPIPE"))
 	if err != nil {
 		return fmt.Errorf("unable to convert _LIBCONTAINER_LOGPIPE: %w", err)
 	}
+	logPipe := os.NewFile(uintptr(logFd), "logpipe")
 
-	logrus.SetOutput(os.NewFile(uintptr(logFD), "logpipe"))
+	logrus.SetOutput(logPipe)
 	logrus.SetFormatter(new(logrus.JSONFormatter))
 	logrus.Debug("child process in init()")
 
 	// Only init processes have FIFOFD.
-	fifofd := -1
+	var fifoFile *os.File
 	envInitType := os.Getenv("_LIBCONTAINER_INITTYPE")
 	it := initType(envInitType)
 	if it == initStandard {
-		envFifoFd := os.Getenv("_LIBCONTAINER_FIFOFD")
-		if fifofd, err = strconv.Atoi(envFifoFd); err != nil {
+		fifoFd, err := strconv.Atoi(os.Getenv("_LIBCONTAINER_FIFOFD"))
+		if err != nil {
 			return fmt.Errorf("unable to convert _LIBCONTAINER_FIFOFD: %w", err)
 		}
+		fifoFile = os.NewFile(uintptr(fifoFd), "initfifo")
 	}
 
 	var consoleSocket *os.File
@@ -212,10 +214,10 @@ func startInitialization() (retErr error) {
 	}
 
 	// If init succeeds, it will not return, hence none of the defers will be called.
-	return containerInit(it, &config, syncPipe, consoleSocket, pidfdSocket, fifofd, logFD, dmzExe)
+	return containerInit(it, &config, syncPipe, consoleSocket, pidfdSocket, fifoFile, logPipe, dmzExe)
 }
 
-func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSocket, pidfdSocket *os.File, fifoFd, logFd int, dmzExe *os.File) error {
+func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSocket, pidfdSocket, fifoFile, logPipe, dmzExe *os.File) error {
 	if err := populateProcessEnvironment(config.Env); err != nil {
 		return err
 	}
@@ -227,7 +229,7 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock
 			consoleSocket: consoleSocket,
 			pidfdSocket:   pidfdSocket,
 			config:        config,
-			logFd:         logFd,
+			logPipe:       logPipe,
 			dmzExe:        dmzExe,
 		}
 		return i.Init()
@@ -238,8 +240,8 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock
 			pidfdSocket:   pidfdSocket,
 			parentPid:     unix.Getppid(),
 			config:        config,
-			fifoFd:        fifoFd,
-			logFd:         logFd,
+			fifoFile:      fifoFile,
+			logPipe:       logPipe,
 			dmzExe:        dmzExe,
 		}
 		return i.Init()

libcontainer/mount_linux.go

@@ -113,12 +113,12 @@ func mountViaFds(source string, srcFile *mountSource, target, dstFd, fstype stri
 		// mount(2), we need to get a safe handle to /proc/thread-self. This
 		// isn't needed for move_mount(2) because in that case the path is just
 		// a dummy string used for error info.
-		fdStr := strconv.Itoa(int(srcFile.file.Fd()))
+		srcFileFd := srcFile.file.Fd()
 		if isMoveMount {
-			src = "/proc/self/fd/" + fdStr
+			src = "/proc/self/fd/" + strconv.Itoa(int(srcFileFd))
 		} else {
 			var closer utils.ProcThreadSelfCloser
-			src, closer = utils.ProcThreadSelf("fd/" + fdStr)
+			src, closer = utils.ProcThreadSelfFd(srcFileFd)
 			defer closer()
 		}
 	}

libcontainer/setns_init_linux.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
-	"strconv"
 
 	"github.com/opencontainers/selinux/go-selinux"
 	"github.com/sirupsen/logrus"
@@ -24,7 +23,7 @@ type linuxSetnsInit struct {
 	consoleSocket *os.File
 	pidfdSocket   *os.File
 	config        *initConfig
-	logFd         int
+	logPipe       *os.File
 	dmzExe        *os.File
 }
 
@@ -131,8 +130,8 @@ func (l *linuxSetnsInit) Init() error {
 
 	// Close the log pipe fd so the parent's ForwardLogs can exit.
 	logrus.Debugf("setns_init: about to exec")
-	if err := unix.Close(l.logFd); err != nil {
-		return &os.PathError{Op: "close log pipe", Path: "fd " + strconv.Itoa(l.logFd), Err: err}
+	if err := l.logPipe.Close(); err != nil {
+		return fmt.Errorf("close log pipe: %w", err)
 	}
 
 	if l.dmzExe != nil {

libcontainer/standard_init_linux.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
-	"strconv"
 
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/selinux/go-selinux"
@@ -25,8 +24,8 @@ type linuxStandardInit struct {
 	consoleSocket *os.File
 	pidfdSocket   *os.File
 	parentPid     int
-	fifoFd        int
-	logFd         int
+	fifoFile      *os.File
+	logPipe       *os.File
 	dmzExe        *os.File
 	config        *initConfig
 }
@@ -244,11 +243,11 @@ func (l *linuxStandardInit) Init() error {
 
 	// Close the log pipe fd so the parent's ForwardLogs can exit.
 	logrus.Debugf("init: about to wait on exec fifo")
-	if err := unix.Close(l.logFd); err != nil {
-		return &os.PathError{Op: "close log pipe", Path: "fd " + strconv.Itoa(l.logFd), Err: err}
+	if err := l.logPipe.Close(); err != nil {
+		return fmt.Errorf("close log pipe: %w", err)
 	}
 
-	fifoPath, closer := utils.ProcThreadSelf("fd/" + strconv.Itoa(l.fifoFd))
+	fifoPath, closer := utils.ProcThreadSelfFd(l.fifoFile.Fd())
 	defer closer()
 
 	// Wait for the FIFO to be opened on the other side before exec-ing the
@@ -269,7 +268,7 @@ func (l *linuxStandardInit) Init() error {
 	// N.B. the core issue itself (passing dirfds to the host filesystem) has
 	// since been resolved.
 	// https://github.com/torvalds/linux/blob/v4.9/fs/exec.c#L1290-L1318
-	_ = unix.Close(l.fifoFd)
+	_ = l.fifoFile.Close()
 
 	s := l.config.SpecState
 	s.Pid = unix.Getpid()

libcontainer/utils/utils_unix.go

@@ -202,3 +202,12 @@ func ProcThreadSelf(subpath string) (string, ProcThreadSelfCloser) {
 	}
 	return threadSelf + subpath, runtime.UnlockOSThread
 }
+
+// ProcThreadSelfFd is small wrapper around ProcThreadSelf to make it easier to
+// create a /proc/thread-self handle for given file descriptor.
+//
+// It is basically equivalent to ProcThreadSelf(fmt.Sprintf("fd/%d", fd)), but
+// without using fmt.Sprintf to avoid unneeded overhead.
+func ProcThreadSelfFd(fd uintptr) (string, ProcThreadSelfCloser) {
+	return ProcThreadSelf("fd/" + strconv.FormatUint(uint64(fd), 10))
+}