Skip to content

Commit 1767d5c

Browse files
committed
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 <golang/go#61751>. It will cause runc init process can't start. (opencontainers#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 <[email protected]>
1 parent d82235c commit 1767d5c

File tree

2 files changed

+33
-8
lines changed

2 files changed

+33
-8
lines changed

libcontainer/container_linux.go

+22-8
Original file line numberDiff line numberDiff line change
@@ -619,15 +619,29 @@ func (c *Container) newParentProcess(p *Process) (parentProcess, error) {
619619
}
620620

621621
if safeExe != nil {
622-
// Due to a Go stdlib bug, we need to add safeExe to the set of
623-
// ExtraFiles otherwise it is possible for the stdlib to clobber the fd
622+
// Due to a Go stdlib bug, it is possible for the stdlib to clobber the fd
624623
// during forkAndExecInChild1 and replace it with some other file that
625-
// might be malicious. This is less than ideal (because the descriptor
626-
// will be non-O_CLOEXEC) however we have protections in "runc init" to
627-
// stop us from leaking extra file descriptors.
628-
//
629-
// See <https://github.com/golang/go/issues/61751>.
630-
cmd.ExtraFiles = append(cmd.ExtraFiles, safeExe)
624+
// might be malicious. See <https://github.com/golang/go/issues/61751>.
625+
// It will cause runc init process can't start. (#4294)
626+
minFd := stdioFdCount + len(cmd.ExtraFiles)
627+
if p.Init {
628+
// If this is init process, we need to attach fifo fd.
629+
minFd++
630+
}
631+
if int(safeExe.Fd()) <= minFd {
632+
maxFd, err := utils.GetMaxFds()
633+
if err != nil {
634+
return nil, fmt.Errorf("unable to get the max opened fd of current process: %w", err)
635+
}
636+
maxFd++
637+
if err := unix.Dup3(int(safeExe.Fd()), maxFd, unix.O_CLOEXEC); err != nil {
638+
return nil, fmt.Errorf("unable to dup3 the fd from %d to %d: %w", safeExe.Fd(), maxFd, err)
639+
}
640+
cmd.Path = "/proc/self/fd/" + strconv.Itoa(maxFd)
641+
if err := safeExe.Close(); err != nil {
642+
return nil, fmt.Errorf("unable to close old safe exe(%d): %w", safeExe.Fd(), err)
643+
}
644+
}
631645
}
632646

633647
// NOTE: when running a container with no PID namespace and the parent

libcontainer/utils/utils_unix.go

+11
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,17 @@ func fdRangeFrom(minFd int, fn fdFunc) error {
9797
return nil
9898
}
9999

100+
// GetMaxFds returns the max opened fd of current process.
101+
func GetMaxFds() (int, error) {
102+
maxFd := -1
103+
err := fdRangeFrom(-1, func(fd int) {
104+
if fd > maxFd {
105+
maxFd = fd
106+
}
107+
})
108+
return maxFd, err
109+
}
110+
100111
// CloseExecFrom sets the O_CLOEXEC flag on all file descriptors greater or
101112
// equal to minFd in the current process.
102113
func CloseExecFrom(minFd int) error {

0 commit comments

Comments
 (0)