From b3be2b0b4f9be3f4fd8819c8db1f970fc9ebf000 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 9 Feb 2021 15:56:25 -0800 Subject: [PATCH 1/2] libct: close execFifo after start Apparently, the parent never closes execFifo fd. Not a problem for runc per se, but can be an issue for a user of libcontainer. Signed-off-by: Kir Kolyshkin --- libcontainer/container_linux.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 93556a6534e..7ce505ea014 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -55,6 +55,7 @@ type linuxContainer struct { criuVersion int state containerState created time.Time + fifo *os.File } // State represents a running container's state @@ -380,6 +381,7 @@ func (c *linuxContainer) start(process *Process) (retErr error) { } if process.Init { + c.fifo.Close() if c.config.Hooks != nil { s, err := c.currentOCIState() if err != nil { @@ -455,12 +457,13 @@ func (c *linuxContainer) deleteExecFifo() { // fd, with _LIBCONTAINER_FIFOFD set to its fd number. func (c *linuxContainer) includeExecFifo(cmd *exec.Cmd) error { fifoName := filepath.Join(c.root, execFifoFilename) - fifoFd, err := unix.Open(fifoName, unix.O_PATH|unix.O_CLOEXEC, 0) + fifo, err := os.OpenFile(fifoName, unix.O_PATH|unix.O_CLOEXEC, 0) if err != nil { return err } + c.fifo = fifo - cmd.ExtraFiles = append(cmd.ExtraFiles, os.NewFile(uintptr(fifoFd), fifoName)) + cmd.ExtraFiles = append(cmd.ExtraFiles, fifo) cmd.Env = append(cmd.Env, "_LIBCONTAINER_FIFOFD="+strconv.Itoa(stdioFdCount+len(cmd.ExtraFiles)-1)) return nil From 79a8647b818c3ad9e25d0325a5df838640a4cdd3 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 9 Feb 2021 19:33:13 -0800 Subject: [PATCH 2/2] libct/int: add TestFdLeaks This is a very simple test that checks that container.Run do not leak opened file descriptors. In fact it does, so we have to add two exclusions: 1. /sys/fs/cgroup is opened once per lifetime in prepareOpenat2(), provided that cgroupv2 is used and openat2 is available. This works as intended ("it's not a bug, it's a feature"). 2. ebpf program fd is leaked every time we call setDevices() for cgroupv2 (iow, every container.Run or container.Set leaks 1 fd). This needs to be fixed, thus FIXME. Signed-off-by: Kir Kolyshkin --- libcontainer/integration/exec_test.go | 61 +++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 783af0efb34..1cb1c88c421 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -1996,3 +1996,64 @@ func TestCGROUPHost(t *testing.T) { t.Fatalf("cgroup link not equal to host link %q %q", actual, l) } } + +func TestFdLeaks(t *testing.T) { + if testing.Short() { + return + } + + rootfs, err := newRootfs() + ok(t, err) + defer remove(rootfs) + + pfd, err := os.Open("/proc/self/fd") + ok(t, err) + defer pfd.Close() + fds0, err := pfd.Readdirnames(0) + ok(t, err) + _, err = pfd.Seek(0, 0) + ok(t, err) + + config := newTemplateConfig(&tParam{rootfs: rootfs}) + buffers, exitCode, err := runContainer(config, "", "true") + ok(t, err) + + if exitCode != 0 { + t.Fatalf("exit code not 0. code %d stderr %q", exitCode, buffers.Stderr) + } + + fds1, err := pfd.Readdirnames(0) + ok(t, err) + + if len(fds1) == len(fds0) { + return + } + // Show the extra opened files. + + excludedPaths := []string{ + "/sys/fs/cgroup", // opened once, see prepareOpenat2 + "anon_inode:bpf-prog", // FIXME: see https://github.com/opencontainers/runc/issues/2366#issuecomment-776411392 + } + + count := 0 +next_fd: + for _, fd1 := range fds1 { + for _, fd0 := range fds0 { + if fd0 == fd1 { + continue next_fd + } + } + dst, _ := os.Readlink("/proc/self/fd/" + fd1) + for _, ex := range excludedPaths { + if ex == dst { + continue next_fd + } + } + + count++ + t.Logf("extra fd %s -> %s", fd1, dst) + } + if count > 0 { + t.Fatalf("found %d extra fds after container.Run", count) + } +}