From 26ce0d7e39c7301aafee57947da04ba87aa8625d Mon Sep 17 00:00:00 2001 From: unclejack Date: Sat, 19 Oct 2013 01:44:06 +0300 Subject: [PATCH 1/4] disallow / as source for bind mount in the cli This makes the docker cli reject docker run commands which include bind mounts like "/:/some/path/in/the/container". Bind mounting the root directory is a bad idea and the cli should throw an error right away. The same check will also be made by the remote API via another commit. Upstream-commit: 4b8c41c4a2591797f97ad079e4314b21498ba732 Component: engine --- components/engine/container.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/components/engine/container.go b/components/engine/container.go index a22484c2d6..92e05283d1 100644 --- a/components/engine/container.go +++ b/components/engine/container.go @@ -251,6 +251,9 @@ func ParseRun(args []string, capabilities *Capabilities) (*Config, *HostConfig, for bind := range flVolumes { arr := strings.Split(bind, ":") if len(arr) > 1 { + if arr[0] == "/" { + return nil, nil, cmd, fmt.Errorf("Invalid bind mount: source can't be '/'") + } dstDir := arr[1] flVolumes[dstDir] = struct{}{} binds = append(binds, bind) From 730dda3e215557d364ffd2b0a920623324df1992 Mon Sep 17 00:00:00 2001 From: unclejack Date: Sat, 19 Oct 2013 01:53:15 +0300 Subject: [PATCH 2/4] add test to ensure / can't be bind mounted This adds a test which checks that we're erroring out when we attempt to bind mount root in a container. Upstream-commit: f1f39616eb052455a383de589bbc1eaa9ab5d96c Component: engine --- components/engine/commands_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/components/engine/commands_test.go b/components/engine/commands_test.go index 2c0e319787..e33ba9c7cc 100644 --- a/components/engine/commands_test.go +++ b/components/engine/commands_test.go @@ -660,4 +660,22 @@ func TestCmdLogs(t *testing.T) { if err := cli.CmdLogs(globalRuntime.List()[0].ID); err != nil { t.Fatal(err) } + +// Expected behaviour: using / as a bind mount source should throw an error +func TestRunErrorBindMountRootSource(t *testing.T) { + + cli := NewDockerCli(nil, nil, ioutil.Discard, testDaemonProto, testDaemonAddr) + defer cleanup(globalRuntime) + + c := make(chan struct{}) + go func() { + defer close(c) + if err := cli.CmdRun("-v", "/:/tmp", unitTestImageID, "echo 'should fail'"); err == nil { + t.Fatal("should have failed to run when using / as a source for the bind mount") + } + }() + + setTimeout(t, "CmdRun timed out", 5*time.Second, func() { + <-c + }) } From 9f8cb6e156a5d37856fd4094b08e85a480126807 Mon Sep 17 00:00:00 2001 From: unclejack Date: Sat, 19 Oct 2013 01:56:52 +0300 Subject: [PATCH 3/4] validate bind mounts on the server side This changes the server side code to make sure that: 1) the source of a bind mount isn't / The bind mount "/:/foo" isn't allowed. 2) Check that the source exists The source to be bind mounted must exist. This fixes issue #2070. Upstream-commit: 4d2ba779e1a0596c51cc6ed2ddb7c2139830f15c Component: engine --- components/engine/server.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/components/engine/server.go b/components/engine/server.go index 314df0256b..cbe7c6435c 100644 --- a/components/engine/server.go +++ b/components/engine/server.go @@ -1316,6 +1316,25 @@ func (srv *Server) RegisterLinks(name string, hostConfig *HostConfig) error { func (srv *Server) ContainerStart(name string, hostConfig *HostConfig) error { runtime := srv.runtime container := runtime.Get(name) + + if hostConfig != nil { + for _, bind := range hostConfig.Binds { + splitBind := strings.Split(bind, ":") + source := splitBind[0] + + // refuse to bind mount "/" to the container + if source == "/" { + return fmt.Errorf("Invalid bind mount '%s' : source can't be '/'", bind) + } + + // ensure the source exists on the host + _, err := os.Stat(source) + if err != nil && os.IsNotExist(err) { + return fmt.Errorf("Invalid bind mount '%s' : source doesn't exist", bind) + } + } + } + if container == nil { return fmt.Errorf("No such container: %s", name) } From 497f4207e979ee57cdcc2efb35cbc4793c6c6f6a Mon Sep 17 00:00:00 2001 From: unclejack Date: Sat, 19 Oct 2013 02:06:25 +0300 Subject: [PATCH 4/4] test: error out when bind mount source doesn't exist This adds a test to verify that the server is checking whether the path to be bind mounted actually exists on the server. Upstream-commit: 35430e89201972d877ba01405f966882a1d3888a Component: engine --- components/engine/commands_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/components/engine/commands_test.go b/components/engine/commands_test.go index e33ba9c7cc..b38ed33132 100644 --- a/components/engine/commands_test.go +++ b/components/engine/commands_test.go @@ -679,3 +679,22 @@ func TestRunErrorBindMountRootSource(t *testing.T) { <-c }) } + +// Expected behaviour: error out when attempting to bind mount non-existing source paths +func TestRunErrorBindNonExistingSource(t *testing.T) { + + cli := NewDockerCli(nil, nil, ioutil.Discard, testDaemonProto, testDaemonAddr) + defer cleanup(globalRuntime) + + c := make(chan struct{}) + go func() { + defer close(c) + if err := cli.CmdRun("-v", "/i/dont/exist:/tmp", unitTestImageID, "echo 'should fail'"); err == nil { + t.Fatal("should have failed to run when using /i/dont/exist as a source for the bind mount") + } + }() + + setTimeout(t, "CmdRun timed out", 5*time.Second, func() { + <-c + }) +}