From eac6e91d7cee715ab526563170773f2a17aba5fb 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 Due to a Go stdlib bug, it is possible for the stdlib to clobber the fd during forkAndExecInChild1 and replace it with some other file that might be malicious. See . It will cause runc init process can't start. (#4294) It only occurs when we are using a fd type string, for example: proc/self/fd/7, as a cmd path to start runc init, because there is a fd reuse race, if some small fd closed, the kernel may reuse this fd to refer to runc binary. If this fd num is small than the length of `cmd.ExtraFiles`, it will hit this Go stdlib bug. If we found this situation, we can dup it as a new bigger fd num to avoid. Signed-off-by: lfbzhm --- libcontainer/container_linux.go | 30 ++++++++++++++++++++++-------- libcontainer/utils/utils_unix.go | 11 +++++++++++ 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 0b17168c2bb..8f407a9d473 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -619,15 +619,29 @@ func (c *Container) newParentProcess(p *Process) (parentProcess, error) { } if safeExe != nil { - // 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 + // Due to a Go stdlib bug, 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 . - cmd.ExtraFiles = append(cmd.ExtraFiles, safeExe) + // might be malicious. See . + // It will cause runc init process can't start. (#4294) + minFd := stdioFdCount + len(cmd.ExtraFiles) + if p.Init { + // If this is init process, we need to attach fifo fd. + minFd++ + } + if int(safeExe.Fd()) <= minFd { + maxFd, err := utils.GetMaxFds() + if err != nil { + return nil, fmt.Errorf("unable to get the max opened fd of current process: %w", err) + } + maxFd++ + 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) + if err := safeExe.Close(); err != nil { + return nil, fmt.Errorf("unable to close old safe exe(%d): %w", safeExe.Fd(), err) + } + } } // 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 c8ad559d931..1c7594f8737 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 {