Browse Source

Don't make container mount unbindable

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Michael Crosby 7 years ago
parent
commit
4c000662fe
2 changed files with 1 additions and 86 deletions
  1. 1 12
      daemon/container_operations_unix.go
  2. 0 74
      integration/container/mounts_linux_test.go

+ 1 - 12
daemon/container_operations_unix.go

@@ -293,7 +293,6 @@ func (daemon *Daemon) createSecretsDir(c *container.Container) error {
 	if err := mount.Mount("tmpfs", dir, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil {
 		return errors.Wrap(err, "unable to setup secret mount")
 	}
-
 	return nil
 }
 
@@ -400,15 +399,5 @@ func (daemon *Daemon) setupContainerMountsRoot(c *container.Container) error {
 	if err != nil {
 		return err
 	}
-
-	if err := idtools.MkdirAllAndChown(p, 0700, daemon.idMappings.RootPair()); err != nil {
-		return err
-	}
-
-	if err := mount.MakeUnbindable(p); err != nil {
-		// Setting unbindable is a precaution and is not neccessary for correct operation.
-		// Do not error out if this fails.
-		logrus.WithError(err).WithField("resource", p).WithField("container", c.ID).Warn("Error setting container resource mounts to unbindable, this may cause mount leakages, preventing removal of this container.")
-	}
-	return nil
+	return idtools.MkdirAllAndChown(p, 0700, daemon.idMappings.RootPair())
 }

+ 0 - 74
integration/container/mounts_linux_test.go

@@ -1,7 +1,6 @@
 package container // import "github.com/docker/docker/integration/container"
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"path/filepath"
@@ -13,8 +12,6 @@ import (
 	"github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/client"
 	"github.com/docker/docker/integration/internal/request"
-	"github.com/docker/docker/internal/test/daemon"
-	"github.com/docker/docker/pkg/stdcopy"
 	"github.com/docker/docker/pkg/system"
 	"github.com/gotestyourself/gotestyourself/assert"
 	is "github.com/gotestyourself/gotestyourself/assert/cmp"
@@ -22,77 +19,6 @@ import (
 	"github.com/gotestyourself/gotestyourself/skip"
 )
 
-func TestContainerShmNoLeak(t *testing.T) {
-	skip.If(t, testEnv.IsRemoteDaemon(), "cannot start daemon on remote test run")
-	t.Parallel()
-	d := daemon.New(t)
-	client, err := d.NewClient()
-	if err != nil {
-		t.Fatal(err)
-	}
-	d.StartWithBusybox(t, "--iptables=false")
-	defer d.Stop(t)
-
-	ctx := context.Background()
-	cfg := container.Config{
-		Image: "busybox",
-		Cmd:   []string{"top"},
-	}
-
-	ctr, err := client.ContainerCreate(ctx, &cfg, nil, nil, "")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer client.ContainerRemove(ctx, ctr.ID, types.ContainerRemoveOptions{Force: true})
-
-	if err := client.ContainerStart(ctx, ctr.ID, types.ContainerStartOptions{}); err != nil {
-		t.Fatal(err)
-	}
-
-	// this should recursively bind mount everything in the test daemons root
-	// except of course we are hoping that the previous containers /dev/shm mount did not leak into this new container
-	hc := container.HostConfig{
-		Mounts: []mount.Mount{
-			{
-				Type:   mount.TypeBind,
-				Source: d.Root,
-				Target: "/testdaemonroot",
-			},
-		},
-	}
-	cfg.Cmd = []string{"/bin/sh", "-c", fmt.Sprintf("mount | grep testdaemonroot | grep containers | grep %s", ctr.ID)}
-	cfg.AttachStdout = true
-	cfg.AttachStderr = true
-	ctrLeak, err := client.ContainerCreate(ctx, &cfg, &hc, nil, "")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	attach, err := client.ContainerAttach(ctx, ctrLeak.ID, types.ContainerAttachOptions{
-		Stream: true,
-		Stdout: true,
-		Stderr: true,
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if err := client.ContainerStart(ctx, ctrLeak.ID, types.ContainerStartOptions{}); err != nil {
-		t.Fatal(err)
-	}
-
-	buf := bytes.NewBuffer(nil)
-
-	if _, err := stdcopy.StdCopy(buf, buf, attach.Reader); err != nil {
-		t.Fatal(err)
-	}
-
-	out := bytes.TrimSpace(buf.Bytes())
-	if !bytes.Equal(out, []byte{}) {
-		t.Fatalf("mount leaked: %s", string(out))
-	}
-}
-
 func TestContainerNetworkMountsNoChown(t *testing.T) {
 	// chown only applies to Linux bind mounted volumes; must be same host to verify
 	skip.If(t, testEnv.DaemonInfo.OSType != "linux" || testEnv.IsRemoteDaemon())