Set FD_CLOEXEC on all open FDs before creating the sandbox or gofer processes.

Inherited FDs can cause conflict while remapping stdio FDs to startingStdioFD
and beyond. It can also cause failures in validateOpenFDs() which ensures no
directory FDs are open.

It is also better from a security perspective to not mistakenly leak file
descriptors from parent to the sandbox process.

Fixes #10232

PiperOrigin-RevId: 622032875
This commit is contained in:
Ayush Ranjan
2024-04-04 18:36:16 -07:00
committed by gVisor bot
parent 1a5bd5cfdf
commit d5e8d33af3
2 changed files with 50 additions and 0 deletions
+5
View File
@@ -1199,6 +1199,11 @@ func (c *Container) createGoferProcess(spec *specs.Spec, conf *config.Config, bu
return []*os.File{ioFile}, nil, nil, nil
}
// Ensure we don't leak FDs to the gofer process.
if err := sandbox.SetCloExeOnAllFDs(); err != nil {
return nil, nil, nil, fmt.Errorf("setting CLOEXEC on all FDs: %w", err)
}
donations := donation.Agency{}
defer donations.Close()
+45
View File
@@ -647,6 +647,11 @@ func (s *Sandbox) connError(err error) error {
// createSandboxProcess starts the sandbox as a subprocess by running the "boot"
// command, passing in the bundle dir.
func (s *Sandbox) createSandboxProcess(conf *config.Config, args *Args, startSyncFile *os.File) error {
// Ensure we don't leak FDs to the sandbox process.
if err := SetCloExeOnAllFDs(); err != nil {
return fmt.Errorf("setting CLOEXEC on all FDs: %w", err)
}
donations := donation.Agency{}
defer donations.Close()
@@ -1703,3 +1708,43 @@ func (s *Sandbox) Mount(cid, fstype, src, dest string) error {
}
return s.call(boot.ContMgrMount, &args, nil)
}
var setCloseExecOnce sync.Once
// SetCloExeOnAllFDs sets CLOEXEC on all FDs in /proc/self/fd. This avoids
// leaking inherited FDs from the parent (caller) to subprocesses created.
func SetCloExeOnAllFDs() (retErr error) {
// Sufficient to do this only once per runsc invocation. Avoid double work.
setCloseExecOnce.Do(func() {
dents, err := os.ReadDir("/proc/self/fd")
if err != nil {
retErr = fmt.Errorf("failed to read /proc/self/fd: %w", err)
return
}
for _, dent := range dents {
fd, err := strconv.Atoi(dent.Name())
if err != nil {
retErr = fmt.Errorf("failed to convert /proc/self/fd entry %q to int: %w", dent.Name(), err)
return
}
flags, _, errno := unix.RawSyscall(unix.SYS_FCNTL, uintptr(fd), unix.F_GETFD, 0)
if errno == unix.EBADF {
// Ignore EBADF, which is possible for the dir FD used for getdents(2).
continue
}
if errno != 0 {
retErr = fmt.Errorf("error getting FD %d: %w", fd, errno)
return
}
if flags&unix.FD_CLOEXEC != 0 {
continue
}
flags |= unix.FD_CLOEXEC
if _, _, errno := unix.RawSyscall(unix.SYS_FCNTL, uintptr(fd), unix.F_SETFD, flags); errno != 0 {
retErr = fmt.Errorf("error setting CLOEXEC: %v", errno)
return
}
}
})
return
}