From 098ea0f41321e21b18846e9cde8b6ab3817b30f1 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 6 Mar 2014 14:10:32 +0100 Subject: [PATCH] libcontainer: Don't use UsetCloseOnExec, it is racy We can't keep file descriptors without close-on-exec except with syscall.ForkLock held, as otherwise they could leak by accident into other children from forks in other threads. Instead we just use Cmd.ExtraFiles which handles all this for us. This fixes https://github.com/dotcloud/docker/issues/4493 Docker-DCO-1.1-Signed-off-by: Alexander Larsson (github: alexlarsson) Upstream-commit: 5c9b28db1853cccdf7a1037eeaad372d12cd68fa Component: engine --- components/engine/execdriver/native/driver.go | 5 +++-- components/engine/pkg/libcontainer/nsinit/command.go | 8 ++++---- components/engine/pkg/libcontainer/nsinit/exec.go | 2 +- components/engine/pkg/libcontainer/nsinit/sync_pipe.go | 2 -- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/components/engine/execdriver/native/driver.go b/components/engine/execdriver/native/driver.go index e7a722f461..1b444c19d1 100644 --- a/components/engine/execdriver/native/driver.go +++ b/components/engine/execdriver/native/driver.go @@ -239,7 +239,7 @@ type dockerCommandFactory struct { // createCommand will return an exec.Cmd with the Cloneflags set to the proper namespaces // defined on the container's configuration and use the current binary as the init with the // args provided -func (d *dockerCommandFactory) Create(container *libcontainer.Container, console string, syncFd uintptr, args []string) *exec.Cmd { +func (d *dockerCommandFactory) Create(container *libcontainer.Container, console string, syncFile *os.File, args []string) *exec.Cmd { // we need to join the rootfs because nsinit will setup the rootfs and chroot initPath := filepath.Join(d.c.Rootfs, d.c.InitPath) @@ -248,7 +248,7 @@ func (d *dockerCommandFactory) Create(container *libcontainer.Container, console initPath, "-driver", DriverName, "-console", console, - "-pipe", fmt.Sprint(syncFd), + "-pipe", "3", "-root", filepath.Join(d.driver.root, d.c.ID), "--", }, args...) @@ -256,6 +256,7 @@ func (d *dockerCommandFactory) Create(container *libcontainer.Container, console // set this to nil so that when we set the clone flags anything else is reset d.c.SysProcAttr = nil system.SetCloneFlags(&d.c.Cmd, uintptr(nsinit.GetNamespaceFlags(container.Namespaces))) + d.c.ExtraFiles = []*os.File{syncFile} d.c.Env = container.Env d.c.Dir = d.c.Rootfs diff --git a/components/engine/pkg/libcontainer/nsinit/command.go b/components/engine/pkg/libcontainer/nsinit/command.go index 8ddf1e7e71..5546065b6d 100644 --- a/components/engine/pkg/libcontainer/nsinit/command.go +++ b/components/engine/pkg/libcontainer/nsinit/command.go @@ -1,7 +1,6 @@ package nsinit import ( - "fmt" "github.com/dotcloud/docker/pkg/libcontainer" "github.com/dotcloud/docker/pkg/system" "os" @@ -12,7 +11,7 @@ import ( // parent processes and creates an *exec.Cmd that will be used to fork/exec the // namespaced init process type CommandFactory interface { - Create(container *libcontainer.Container, console string, syncFd uintptr, args []string) *exec.Cmd + Create(container *libcontainer.Container, console string, syncFd *os.File, args []string) *exec.Cmd } type DefaultCommandFactory struct { @@ -22,16 +21,17 @@ type DefaultCommandFactory struct { // Create will return an exec.Cmd with the Cloneflags set to the proper namespaces // defined on the container's configuration and use the current binary as the init with the // args provided -func (c *DefaultCommandFactory) Create(container *libcontainer.Container, console string, pipe uintptr, args []string) *exec.Cmd { +func (c *DefaultCommandFactory) Create(container *libcontainer.Container, console string, pipe *os.File, args []string) *exec.Cmd { // get our binary name from arg0 so we can always reexec ourself command := exec.Command(os.Args[0], append([]string{ "-console", console, - "-pipe", fmt.Sprint(pipe), + "-pipe", "3", "-root", c.Root, "init"}, args...)...) system.SetCloneFlags(command, uintptr(GetNamespaceFlags(container.Namespaces))) command.Env = container.Env + command.ExtraFiles = []*os.File{pipe} return command } diff --git a/components/engine/pkg/libcontainer/nsinit/exec.go b/components/engine/pkg/libcontainer/nsinit/exec.go index f1a4e2477a..4963f126e9 100644 --- a/components/engine/pkg/libcontainer/nsinit/exec.go +++ b/components/engine/pkg/libcontainer/nsinit/exec.go @@ -35,7 +35,7 @@ func (ns *linuxNs) Exec(container *libcontainer.Container, term Terminal, args [ term.SetMaster(master) } - command := ns.commandFactory.Create(container, console, syncPipe.child.Fd(), args) + command := ns.commandFactory.Create(container, console, syncPipe.child, args) if err := term.Attach(command); err != nil { return -1, err } diff --git a/components/engine/pkg/libcontainer/nsinit/sync_pipe.go b/components/engine/pkg/libcontainer/nsinit/sync_pipe.go index 7b29e98680..f724f525f0 100644 --- a/components/engine/pkg/libcontainer/nsinit/sync_pipe.go +++ b/components/engine/pkg/libcontainer/nsinit/sync_pipe.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "github.com/dotcloud/docker/pkg/libcontainer" - "github.com/dotcloud/docker/pkg/system" "io/ioutil" "os" ) @@ -22,7 +21,6 @@ func NewSyncPipe() (s *SyncPipe, err error) { if err != nil { return nil, err } - system.UsetCloseOnExec(s.child.Fd()) return s, nil }