From 45c560eac6c3ca846d9ec484d49a5dd738b2dc7d Mon Sep 17 00:00:00 2001 From: lfbzhm Date: Thu, 17 Oct 2024 17:48:23 +0000 Subject: [PATCH] fix an error caused by fd reuse race when starting runc init In #3987(0e9a335), we may use a memfd to copy run to start runc init, due to a Go stdlib bug, we need to add safeExe to the set of ExtraFiles otherwise it is possible for the stdlib to clobber the fd during forkAndExecInChild1 and replace it with some other file that might be malicious. This is less than ideal (because the descriptor will be non-O_CLOEXEC) however we have protections in "runc init" to stop us from leaking extra file descriptors. See . There is a race situation when we are opening this memfd, if the fd 5 or 6 was closed at that time, maybe it will be reused by memfd. But because of we have added safeExe to the set of ExtraFiles, if the fd of safeExe is than stdio fds count + ExtraFiles count, go stdlib will dup3 it to another fd, then it will cause the original fd closed. (#4294) Signed-off-by: lfbzhm --- libcontainer/container_linux.go | 15 +++++++++++++++ libcontainer/utils/utils_unix.go | 11 +++++++++++ 2 files changed, 26 insertions(+) diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 0b17168c2bb..6d0ecdb2177 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -628,6 +628,21 @@ func (c *Container) newParentProcess(p *Process) (parentProcess, error) { // // See . cmd.ExtraFiles = append(cmd.ExtraFiles, safeExe) + + // But because of we have added safeExe to the set of ExtraFiles, if the + // fd of safeExe is too small, go stdlib will dup3 it to another fd, then + // it will cause the original fd closed. (#4294) + if int(safeExe.Fd()) <= stdioFdCount+len(cmd.ExtraFiles) { + maxFd, err := utils.GetMaxFds() + if err != nil { + return nil, fmt.Errorf("unable to get the max opened fd of current process: %w", err) + } + maxFd = maxFd + 1 + if err := unix.Dup3(int(safeExe.Fd()), maxFd, unix.O_CLOEXEC); err != nil { + return nil, fmt.Errorf("unable to dup3 the fd from %d to %d: %w", safeExe.Fd(), maxFd, err) + } + cmd.Path = "/proc/self/fd/" + strconv.Itoa(maxFd) + } } // NOTE: when running a container with no PID namespace and the parent diff --git a/libcontainer/utils/utils_unix.go b/libcontainer/utils/utils_unix.go index cc84597a7ce..e76b5f17e5a 100644 --- a/libcontainer/utils/utils_unix.go +++ b/libcontainer/utils/utils_unix.go @@ -97,6 +97,17 @@ func fdRangeFrom(minFd int, fn fdFunc) error { return nil } +// GetMaxFds returns the max opened fd of current process +func GetMaxFds() (int, error) { + maxFd := -1 + err := fdRangeFrom(-1, func(fd int) { + if fd > maxFd { + maxFd = fd + } + }) + return maxFd, err +} + // CloseExecFrom sets the O_CLOEXEC flag on all file descriptors greater or // equal to minFd in the current process. func CloseExecFrom(minFd int) error {