Browse Source

Fix bindmount autocreate race

When using the mounts API, bind mounts are not supposed to be
automatically created.

Before this patch there is a race condition between valiating that a
bind path exists and then actually setting up the bind mount where the
bind path may exist during validation but was removed during mountpooint
setup.

This adds a field to the mountpoint struct to ensure that binds created
over the mounts API are not accidentally created.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
Brian Goff 7 years ago
parent
commit
1caeb79963
2 changed files with 15 additions and 0 deletions
  1. 4 0
      daemon/volumes.go
  2. 11 0
      volume/mounts/mounts.go

+ 4 - 0
daemon/volumes.go

@@ -215,6 +215,10 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
 			}
 			}
 		}
 		}
 
 
+		if mp.Type == mounttypes.TypeBind {
+			mp.SkipMountpointCreation = true
+		}
+
 		binds[mp.Destination] = true
 		binds[mp.Destination] = true
 		dereferenceIfExists(mp.Destination)
 		dereferenceIfExists(mp.Destination)
 		mountPoints[mp.Destination] = mp
 		mountPoints[mp.Destination] = mp

+ 11 - 0
volume/mounts/mounts.go

@@ -62,6 +62,12 @@ type MountPoint struct {
 	// Sepc is a copy of the API request that created this mount.
 	// Sepc is a copy of the API request that created this mount.
 	Spec mounttypes.Mount
 	Spec mounttypes.Mount
 
 
+	// Some bind mounts should not be automatically created.
+	// (Some are auto-created for backwards-compatability)
+	// This is checked on the API but setting this here prevents race conditions.
+	// where a bind dir existed during validation was removed before reaching the setup code.
+	SkipMountpointCreation bool
+
 	// Track usage of this mountpoint
 	// Track usage of this mountpoint
 	// Specifically needed for containers which are running and calls to `docker cp`
 	// Specifically needed for containers which are running and calls to `docker cp`
 	// because both these actions require mounting the volumes.
 	// because both these actions require mounting the volumes.
@@ -90,6 +96,10 @@ func (m *MountPoint) Cleanup() error {
 // The, optional, checkFun parameter allows doing additional checking
 // The, optional, checkFun parameter allows doing additional checking
 // before creating the source directory on the host.
 // before creating the source directory on the host.
 func (m *MountPoint) Setup(mountLabel string, rootIDs idtools.IDPair, checkFun func(m *MountPoint) error) (path string, err error) {
 func (m *MountPoint) Setup(mountLabel string, rootIDs idtools.IDPair, checkFun func(m *MountPoint) error) (path string, err error) {
+	if m.SkipMountpointCreation {
+		return m.Source, nil
+	}
+
 	defer func() {
 	defer func() {
 		if err != nil || !label.RelabelNeeded(m.Mode) {
 		if err != nil || !label.RelabelNeeded(m.Mode) {
 			return
 			return
@@ -140,6 +150,7 @@ func (m *MountPoint) Setup(mountLabel string, rootIDs idtools.IDPair, checkFun f
 				return "", err
 				return "", err
 			}
 			}
 		}
 		}
+
 		// idtools.MkdirAllNewAs() produces an error if m.Source exists and is a file (not a directory)
 		// idtools.MkdirAllNewAs() produces an error if m.Source exists and is a file (not a directory)
 		// also, makes sure that if the directory is created, the correct remapped rootUID/rootGID will own it
 		// also, makes sure that if the directory is created, the correct remapped rootUID/rootGID will own it
 		if err := idtools.MkdirAllAndChownNew(m.Source, 0755, rootIDs); err != nil {
 		if err := idtools.MkdirAllAndChownNew(m.Source, 0755, rootIDs); err != nil {