From 1caeb79963d3c9f770b23be2f12c584adf49538d Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 2 Jul 2018 12:02:54 -0700 Subject: [PATCH] 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 --- daemon/volumes.go | 4 ++++ volume/mounts/mounts.go | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/daemon/volumes.go b/daemon/volumes.go index a20ff1fbf5..d1c98d0a4f 100644 --- a/daemon/volumes.go +++ b/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 dereferenceIfExists(mp.Destination) mountPoints[mp.Destination] = mp diff --git a/volume/mounts/mounts.go b/volume/mounts/mounts.go index 8f255a5482..177f4c3d97 100644 --- a/volume/mounts/mounts.go +++ b/volume/mounts/mounts.go @@ -62,6 +62,12 @@ type MountPoint struct { // Sepc is a copy of the API request that created this 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 // Specifically needed for containers which are running and calls to `docker cp` // 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 // 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) { + if m.SkipMountpointCreation { + return m.Source, nil + } + defer func() { if err != nil || !label.RelabelNeeded(m.Mode) { return @@ -140,6 +150,7 @@ func (m *MountPoint) Setup(mountLabel string, rootIDs idtools.IDPair, checkFun f return "", err } } + // 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 if err := idtools.MkdirAllAndChownNew(m.Source, 0755, rootIDs); err != nil {