Ensure daemon root is unmounted on shutdown

This is only for the case when dockerd has had to re-mount the daemon
root as shared.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

daemon/daemon_linux.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/docker/docker/pkg/fileutils"
 	"github.com/docker/docker/pkg/mount"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
@@ -67,9 +68,29 @@ func (daemon *Daemon) cleanupMountsFromReaderByID(reader io.Reader, id string, u
 	return nil
 }
 
-// cleanupMounts umounts shm/mqueue mounts for old containers
+// cleanupMounts umounts used by container resources and the daemon root mount
 func (daemon *Daemon) cleanupMounts() error {
-	return daemon.cleanupMountsByID("")
+	if err := daemon.cleanupMountsByID(""); err != nil {
+		return err
+	}
+
+	infos, err := mount.GetMounts()
+	if err != nil {
+		return errors.Wrap(err, "error reading mount table for cleanup")
+	}
+
+	info := getMountInfo(infos, daemon.root)
+	// `info.Root` here is the root mountpoint of the passed in path (`daemon.root`).
+	// The ony cases that need to be cleaned up is when the daemon has performed a
+	//   `mount --bind /daemon/root /daemon/root && mount --make-shared /daemon/root`
+	// This is only done when the daemon is started up and `/daemon/root` is not
+	// already on a shared mountpoint.
+	if !shouldUnmountRoot(daemon.root, info) {
+		return nil
+	}
+
+	logrus.WithField("mountpoint", daemon.root).Debug("unmounting daemon root")
+	return mount.Unmount(daemon.root)
 }
 
 func getCleanPatterns(id string) (regexps []*regexp.Regexp) {
@@ -91,3 +112,16 @@ func getCleanPatterns(id string) (regexps []*regexp.Regexp) {
 func getRealPath(path string) (string, error) {
 	return fileutils.ReadSymlinkedDirectory(path)
 }
+
+func shouldUnmountRoot(root string, info *mount.Info) bool {
+	if info == nil {
+		return false
+	}
+	if info.Mountpoint != root {
+		return false
+	}
+	if !strings.HasSuffix(root, info.Root) {
+		return false
+	}
+	return hasMountinfoOption(info.Optional, sharedPropagationOption)
+}

daemon/daemon_linux_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/oci"
 	"github.com/docker/docker/pkg/idtools"
+	"github.com/docker/docker/pkg/mount"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -164,3 +165,66 @@ func TestValidateContainerIsolationLinux(t *testing.T) {
 	_, err := d.verifyContainerSettings("linux", &containertypes.HostConfig{Isolation: containertypes.IsolationHyperV}, nil, false)
 	assert.EqualError(t, err, "invalid isolation 'hyperv' on linux")
 }
+
+func TestShouldUnmountRoot(t *testing.T) {
+	for _, test := range []struct {
+		desc   string
+		root   string
+		info   *mount.Info
+		expect bool
+	}{
+		{
+			desc:   "root is at /",
+			root:   "/docker",
+			info:   &mount.Info{Root: "/docker", Mountpoint: "/docker"},
+			expect: true,
+		},
+		{
+			desc:   "not a mountpoint",
+			root:   "/docker",
+			info:   nil,
+			expect: false,
+		},
+		{
+			desc:   "root is at in a submount from `/`",
+			root:   "/foo/docker",
+			info:   &mount.Info{Root: "/docker", Mountpoint: "/foo/docker"},
+			expect: true,
+		},
+		{
+			desc:   "root is mounted in from a parent mount namespace same root dir", // dind is an example of this
+			root:   "/docker",
+			info:   &mount.Info{Root: "/docker/volumes/1234657/_data", Mountpoint: "/docker"},
+			expect: false,
+		},
+		{
+			desc:   "root is mounted in from a parent mount namespace different root dir",
+			root:   "/foo/bar",
+			info:   &mount.Info{Root: "/docker/volumes/1234657/_data", Mountpoint: "/foo/bar"},
+			expect: false,
+		},
+	} {
+		t.Run(test.desc, func(t *testing.T) {
+			for _, options := range []struct {
+				desc     string
+				Optional string
+				expect   bool
+			}{
+				{desc: "shared", Optional: "shared:", expect: true},
+				{desc: "slave", Optional: "slave:", expect: false},
+				{desc: "private", Optional: "private:", expect: false},
+			} {
+				t.Run(options.desc, func(t *testing.T) {
+					expect := options.expect
+					if expect {
+						expect = test.expect
+					}
+					if test.info != nil {
+						test.info.Optional = options.Optional
+					}
+					assert.Equal(t, expect, shouldUnmountRoot(test.root, test.info))
+				})
+			}
+		})
+	}
+}

daemon/oci_linux.go

@@ -24,6 +24,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/devices"
 	"github.com/opencontainers/runc/libcontainer/user"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
@@ -419,52 +420,46 @@ func getSourceMount(source string) (string, string, error) {
 	return "", "", fmt.Errorf("Could not find source mount of %s", source)
 }
 
+const (
+	sharedPropagationOption = "shared:"
+	slavePropagationOption  = "master:"
+)
+
+// hasMountinfoOption checks if any of the passed any of the given option values
+// are set in the passed in option string.
+func hasMountinfoOption(opts string, vals ...string) bool {
+	for _, opt := range strings.Split(opts, " ") {
+		for _, val := range vals {
+			if strings.HasPrefix(opt, val) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // Ensure mount point on which path is mounted, is shared.
 func ensureShared(path string) error {
-	sharedMount := false
-
 	sourceMount, optionalOpts, err := getSourceMount(path)
 	if err != nil {
 		return err
 	}
 	// Make sure source mount point is shared.
-	optsSplit := strings.Split(optionalOpts, " ")
+	if !hasMountinfoOption(optionalOpts, sharedPropagationOption) {
-	for _, opt := range optsSplit {
+		return errors.Errorf("path %s is mounted on %s but it is not a shared mount", path, sourceMount)
-		if strings.HasPrefix(opt, "shared:") {
-			sharedMount = true
-			break
-		}
-	}
-
-	if !sharedMount {
-		return fmt.Errorf("path %s is mounted on %s but it is not a shared mount", path, sourceMount)
 	}
 	return nil
 }
 
 // Ensure mount point on which path is mounted, is either shared or slave.
 func ensureSharedOrSlave(path string) error {
-	sharedMount := false
-	slaveMount := false
-
 	sourceMount, optionalOpts, err := getSourceMount(path)
 	if err != nil {
 		return err
 	}
-	// Make sure source mount point is shared.
-	optsSplit := strings.Split(optionalOpts, " ")
-	for _, opt := range optsSplit {
-		if strings.HasPrefix(opt, "shared:") {
-			sharedMount = true
-			break
-		} else if strings.HasPrefix(opt, "master:") {
-			slaveMount = true
-			break
-		}
-	}
 
-	if !sharedMount && !slaveMount {
+	if !hasMountinfoOption(optionalOpts, sharedPropagationOption, slavePropagationOption) {
-		return fmt.Errorf("path %s is mounted on %s but it is not a shared or slave mount", path, sourceMount)
+		return errors.Errorf("path %s is mounted on %s but it is not a shared or slave mount", path, sourceMount)
 	}
 	return nil
 }