Browse Source

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 <alexl@redhat.com> (github: alexlarsson)
Alexander Larsson 11 years ago
parent
commit
5c9b28db18

+ 3 - 2
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
 // 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
 // defined on the container's configuration and use the current binary as the init with the
 // args provided
 // 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
 	// we need to join the rootfs because nsinit will setup the rootfs and chroot
 	initPath := filepath.Join(d.c.Rootfs, d.c.InitPath)
 	initPath := filepath.Join(d.c.Rootfs, d.c.InitPath)
 
 
@@ -248,7 +248,7 @@ func (d *dockerCommandFactory) Create(container *libcontainer.Container, console
 		initPath,
 		initPath,
 		"-driver", DriverName,
 		"-driver", DriverName,
 		"-console", console,
 		"-console", console,
-		"-pipe", fmt.Sprint(syncFd),
+		"-pipe", "3",
 		"-root", filepath.Join(d.driver.root, d.c.ID),
 		"-root", filepath.Join(d.driver.root, d.c.ID),
 		"--",
 		"--",
 	}, args...)
 	}, 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
 	// set this to nil so that when we set the clone flags anything else is reset
 	d.c.SysProcAttr = nil
 	d.c.SysProcAttr = nil
 	system.SetCloneFlags(&d.c.Cmd, uintptr(nsinit.GetNamespaceFlags(container.Namespaces)))
 	system.SetCloneFlags(&d.c.Cmd, uintptr(nsinit.GetNamespaceFlags(container.Namespaces)))
+	d.c.ExtraFiles = []*os.File{syncFile}
 
 
 	d.c.Env = container.Env
 	d.c.Env = container.Env
 	d.c.Dir = d.c.Rootfs
 	d.c.Dir = d.c.Rootfs

+ 4 - 4
pkg/libcontainer/nsinit/command.go

@@ -1,7 +1,6 @@
 package nsinit
 package nsinit
 
 
 import (
 import (
-	"fmt"
 	"github.com/dotcloud/docker/pkg/libcontainer"
 	"github.com/dotcloud/docker/pkg/libcontainer"
 	"github.com/dotcloud/docker/pkg/system"
 	"github.com/dotcloud/docker/pkg/system"
 	"os"
 	"os"
@@ -12,7 +11,7 @@ import (
 // parent processes and creates an *exec.Cmd that will be used to fork/exec the
 // parent processes and creates an *exec.Cmd that will be used to fork/exec the
 // namespaced init process
 // namespaced init process
 type CommandFactory interface {
 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 {
 type DefaultCommandFactory struct {
@@ -22,16 +21,17 @@ type DefaultCommandFactory struct {
 // Create will return an exec.Cmd with the Cloneflags set to the proper namespaces
 // 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
 // defined on the container's configuration and use the current binary as the init with the
 // args provided
 // 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
 	// get our binary name from arg0 so we can always reexec ourself
 	command := exec.Command(os.Args[0], append([]string{
 	command := exec.Command(os.Args[0], append([]string{
 		"-console", console,
 		"-console", console,
-		"-pipe", fmt.Sprint(pipe),
+		"-pipe", "3",
 		"-root", c.Root,
 		"-root", c.Root,
 		"init"}, args...)...)
 		"init"}, args...)...)
 
 
 	system.SetCloneFlags(command, uintptr(GetNamespaceFlags(container.Namespaces)))
 	system.SetCloneFlags(command, uintptr(GetNamespaceFlags(container.Namespaces)))
 	command.Env = container.Env
 	command.Env = container.Env
+	command.ExtraFiles = []*os.File{pipe}
 	return command
 	return command
 }
 }
 
 

+ 1 - 1
pkg/libcontainer/nsinit/exec.go

@@ -35,7 +35,7 @@ func (ns *linuxNs) Exec(container *libcontainer.Container, term Terminal, args [
 		term.SetMaster(master)
 		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 {
 	if err := term.Attach(command); err != nil {
 		return -1, err
 		return -1, err
 	}
 	}

+ 0 - 2
pkg/libcontainer/nsinit/sync_pipe.go

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"encoding/json"
 	"fmt"
 	"fmt"
 	"github.com/dotcloud/docker/pkg/libcontainer"
 	"github.com/dotcloud/docker/pkg/libcontainer"
-	"github.com/dotcloud/docker/pkg/system"
 	"io/ioutil"
 	"io/ioutil"
 	"os"
 	"os"
 )
 )
@@ -22,7 +21,6 @@ func NewSyncPipe() (s *SyncPipe, err error) {
 	if err != nil {
 	if err != nil {
 		return nil, err
 		return nil, err
 	}
 	}
-	system.UsetCloseOnExec(s.child.Fd())
 	return s, nil
 	return s, nil
 }
 }