Skip to content

Commit

Permalink
fix a fd resue race when starting runc init
Browse files Browse the repository at this point in the history
In opencontainers#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 <golang/go#61751>.

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. (opencontainers#4294)

Signed-off-by: lfbzhm <[email protected]>
  • Loading branch information
lifubang committed Oct 17, 2024
1 parent d82235c commit 36a964f
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 0 deletions.
15 changes: 15 additions & 0 deletions libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,21 @@ func (c *Container) newParentProcess(p *Process) (parentProcess, error) {
//
// See <https://github.com/golang/go/issues/61751>.
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
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 36a964f

Please sign in to comment.