Skip to content

Commit e502359

Browse files
committed
fix an error caused by fd reuse race when starting runc init
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>. There is a race situation when we are opening this memfd, if the fd 6 or 7 was closed at that time, maybe it will be reused by memfd. Because we want to add safeExe to the set of ExtraFiles, if the fd of safeExe is too small, go stdlib will dup3 it to another fd, or dup3 a other fd to this fd, then it will cause the fd type cmd.Path refers to a random path. (issue: opencontainers#4294) Signed-off-by: lfbzhm <[email protected]>
1 parent d82235c commit e502359

File tree

2 files changed

+38
-0
lines changed

2 files changed

+38
-0
lines changed

libcontainer/container_linux.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -619,6 +619,33 @@ func (c *Container) newParentProcess(p *Process) (parentProcess, error) {
619619
}
620620

621621
if safeExe != nil {
622+
// Because we want to add safeExe to the set of ExtraFiles, if the fd of safeExe is
623+
// too small, go stdlib will dup3 it to another fd, or dup3 a other fd to this fd,
624+
// then it will cause the fd type cmd.Path refers to a random path. (#4294)
625+
626+
// +1 means safeExe.
627+
minFd := stdioFdCount + len(cmd.ExtraFiles) + 1
628+
if p.Init {
629+
// This refers to fifo.
630+
minFd++
631+
}
632+
if int(safeExe.Fd()) <= minFd {
633+
maxFd, err := utils.GetMaxFds()
634+
if err != nil {
635+
return nil, fmt.Errorf("unable to get the max opened fd of current process: %w", err)
636+
}
637+
maxFd++
638+
if err := unix.Dup3(int(safeExe.Fd()), maxFd, unix.O_CLOEXEC); err != nil {
639+
return nil, fmt.Errorf("unable to dup3 the fd from %d to %d: %w", safeExe.Fd(), maxFd, err)
640+
}
641+
cmd.Path = "/proc/self/fd/" + strconv.Itoa(maxFd)
642+
newSafeExe := os.NewFile(uintptr(maxFd), safeExe.Name())
643+
if err := safeExe.Close(); err != nil {
644+
return nil, fmt.Errorf("unable to close old safe exe(%d): %w", safeExe.Fd(), err)
645+
}
646+
safeExe = newSafeExe
647+
}
648+
622649
// Due to a Go stdlib bug, we need to add safeExe to the set of
623650
// ExtraFiles otherwise it is possible for the stdlib to clobber the fd
624651
// during forkAndExecInChild1 and replace it with some other file that

libcontainer/utils/utils_unix.go

Lines changed: 11 additions & 0 deletions
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)