init: use *os.File for passed file descriptors

While it doesn't make much of a practical difference, it seems far more
reasonable to use os.NewFile to wrap all of our passed file descriptors
to make sure they're tracked by the Go runtime and that we don't
double-close them.

Signed-off-by: Aleksa Sarai <[email protected]>

Author: cyphar

libcontainer/init_linux.go

@@ -137,24 +137,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
@@ -208,10 +210,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
 	}
@@ -223,7 +225,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()
@@ -234,8 +236,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))
+}