Skip to content

Commit

Permalink
fix an error caused by fd reuse race when starting runc init
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
lifubang committed Oct 21, 2024
1 parent d82235c commit 1767d5c
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 8 deletions.
30 changes: 22 additions & 8 deletions libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://github.com/golang/go/issues/61751>.
cmd.ExtraFiles = append(cmd.ExtraFiles, safeExe)
// might be malicious. See <https://github.com/golang/go/issues/61751>.
// 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
Expand Down
11 changes: 11 additions & 0 deletions libcontainer/utils/utils_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 1767d5c

Please sign in to comment.