diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index 0117ace59..c46cf0ddf 100644 --- a/libcontainer/init_linux.go +++ b/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() diff --git a/libcontainer/mount_linux.go b/libcontainer/mount_linux.go index 285e03fa4..f9b1adf51 100644 --- a/libcontainer/mount_linux.go +++ b/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() } } diff --git a/libcontainer/setns_init_linux.go b/libcontainer/setns_init_linux.go index 0dd72f95e..b88c6136e 100644 --- a/libcontainer/setns_init_linux.go +++ b/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 { diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index 3096d0d81..dbaf30f02 100644 --- a/libcontainer/standard_init_linux.go +++ b/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() diff --git a/libcontainer/utils/utils_unix.go b/libcontainer/utils/utils_unix.go index a48221b00..3e16d1efb 100644 --- a/libcontainer/utils/utils_unix.go +++ b/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)) +}