From 7830091c4bc65fef4a3e3c9aaf8ac475b83767dc Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Mon, 20 Nov 2017 21:33:11 -0800 Subject: [PATCH] graphdriver/overlay{,2}: remove 'merged' on umount This removes and recreates the merged dir with each umount/mount respectively. This is done to make the impact of leaking mountpoints have less user-visible impact. It's fairly easy to accidentally leak mountpoints (even if moby doesn't, other tools on linux like 'unshare' are quite able to incidentally do so). As of recently, overlayfs reacts to these mounts being leaked (see One trick to force an unmount is to remove the mounted directory and recreate it. Devicemapper now does this, overlay can follow suit. Signed-off-by: Euan Kemp Upstream-commit: af0d589623eff9f8cefced8b527dbd7cf221ce61 Component: engine --- .../daemon/graphdriver/overlay/overlay.go | 39 +++++++++++++------ .../daemon/graphdriver/overlay2/overlay.go | 32 +++++++++++---- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/components/engine/daemon/graphdriver/overlay/overlay.go b/components/engine/daemon/graphdriver/overlay/overlay.go index ff4e300321..cd98d4072f 100644 --- a/components/engine/daemon/graphdriver/overlay/overlay.go +++ b/components/engine/daemon/graphdriver/overlay/overlay.go @@ -296,9 +296,6 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) (retErr if err := idtools.MkdirAndChown(path.Join(dir, "work"), 0700, root); err != nil { return err } - if err := idtools.MkdirAndChown(path.Join(dir, "merged"), 0700, root); err != nil { - return err - } if err := ioutil.WriteFile(path.Join(dir, "lower-id"), []byte(parent), 0666); err != nil { return err } @@ -329,9 +326,6 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) (retErr if err := idtools.MkdirAndChown(path.Join(dir, "work"), 0700, root); err != nil { return err } - if err := idtools.MkdirAndChown(path.Join(dir, "merged"), 0700, root); err != nil { - return err - } return copy.DirCopy(parentUpperDir, upperDir, copy.Content) } @@ -360,6 +354,7 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, err erro if _, err := os.Stat(rootDir); err == nil { return containerfs.NewLocalContainerFS(rootDir), nil } + mergedDir := path.Join(dir, "merged") if count := d.ctr.Increment(mergedDir); count > 1 { return containerfs.NewLocalContainerFS(mergedDir), nil @@ -367,7 +362,13 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, err erro defer func() { if err != nil { if c := d.ctr.Decrement(mergedDir); c <= 0 { - unix.Unmount(mergedDir, 0) + if mntErr := unix.Unmount(mergedDir, 0); mntErr != nil { + logrus.Debugf("Failed to unmount %s: %v: %v", id, mntErr, err) + } + // Cleanup the created merged directory; see the comment in Put's rmdir + if rmErr := unix.Rmdir(mergedDir); rmErr != nil && !os.IsNotExist(rmErr) { + logrus.Warnf("Failed to remove %s: %v: %v", id, rmErr, err) + } } } }() @@ -375,6 +376,13 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, err erro if err != nil { return nil, err } + rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) + if err != nil { + return nil, err + } + if err := idtools.MkdirAndChown(mergedDir, 0700, idtools.IDPair{rootUID, rootGID}); err != nil { + return nil, err + } var ( lowerDir = path.Join(d.dir(string(lowerID)), "root") upperDir = path.Join(dir, "upper") @@ -386,10 +394,6 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, err erro } // chown "workdir/work" to the remapped root UID/GID. Overlay fs inside a // user namespace requires this to move a directory from lower to upper. - rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) - if err != nil { - return nil, err - } if err := os.Chown(path.Join(workDir, "work"), rootUID, rootGID); err != nil { return nil, err } @@ -397,6 +401,8 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, err erro } // Put unmounts the mount path created for the give id. +// It also removes the 'merged' directory to force the kernel to unmount the +// overlay mount in other namespaces. func (d *Driver) Put(id string) error { d.locker.Lock(id) defer d.locker.Unlock(id) @@ -411,6 +417,17 @@ func (d *Driver) Put(id string) error { if err := unix.Unmount(mountpoint, unix.MNT_DETACH); err != nil { logrus.Debugf("Failed to unmount %s overlay: %v", id, err) } + + // Remove the mountpoint here. Removing the mountpoint (in newer kernels) + // will cause all other instances of this mount in other mount namespaces + // to be unmounted. This is necessary to avoid cases where an overlay mount + // that is present in another namespace will cause subsequent mounts + // operations to fail with ebusy. We ignore any errors here because this may + // fail on older kernels which don't have + // torvalds/linux@8ed936b5671bfb33d89bc60bdcc7cf0470ba52fe applied. + if err := unix.Rmdir(mountpoint); err != nil { + logrus.Debugf("Failed to remove %s overlay: %v", id, err) + } return nil } diff --git a/components/engine/daemon/graphdriver/overlay2/overlay.go b/components/engine/daemon/graphdriver/overlay2/overlay.go index 9fea3c48b8..9e27497557 100644 --- a/components/engine/daemon/graphdriver/overlay2/overlay.go +++ b/components/engine/daemon/graphdriver/overlay2/overlay.go @@ -417,9 +417,6 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts) (retErr if err := idtools.MkdirAndChown(path.Join(dir, "work"), 0700, root); err != nil { return err } - if err := idtools.MkdirAndChown(path.Join(dir, "merged"), 0700, root); err != nil { - return err - } lower, err := d.getLower(parent) if err != nil { @@ -548,6 +545,10 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, retErr e if mntErr := unix.Unmount(mergedDir, 0); mntErr != nil { logrus.Errorf("error unmounting %v: %v", mergedDir, mntErr) } + // Cleanup the created merged directory; see the comment in Put's rmdir + if rmErr := unix.Rmdir(mergedDir); rmErr != nil && !os.IsNotExist(rmErr) { + logrus.Debugf("Failed to remove %s: %v: %v", id, rmErr, err) + } } } }() @@ -563,6 +564,14 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, retErr e mount := unix.Mount mountTarget := mergedDir + rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) + if err != nil { + return nil, err + } + if err := idtools.MkdirAndChown(mergedDir, 0700, idtools.IDPair{rootUID, rootGID}); err != nil { + return nil, err + } + pageSize := unix.Getpagesize() // Go can return a larger page size than supported by the system @@ -597,11 +606,6 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, retErr e // chown "workdir/work" to the remapped root UID/GID. Overlay fs inside a // user namespace requires this to move a directory from lower to upper. - rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) - if err != nil { - return nil, err - } - if err := os.Chown(path.Join(workDir, "work"), rootUID, rootGID); err != nil { return nil, err } @@ -610,6 +614,8 @@ func (d *Driver) Get(id, mountLabel string) (_ containerfs.ContainerFS, retErr e } // Put unmounts the mount path created for the give id. +// It also removes the 'merged' directory to force the kernel to unmount the +// overlay mount in other namespaces. func (d *Driver) Put(id string) error { d.locker.Lock(id) defer d.locker.Unlock(id) @@ -630,6 +636,16 @@ func (d *Driver) Put(id string) error { if err := unix.Unmount(mountpoint, unix.MNT_DETACH); err != nil { logrus.Debugf("Failed to unmount %s overlay: %s - %v", id, mountpoint, err) } + // Remove the mountpoint here. Removing the mountpoint (in newer kernels) + // will cause all other instances of this mount in other mount namespaces + // to be unmounted. This is necessary to avoid cases where an overlay mount + // that is present in another namespace will cause subsequent mounts + // operations to fail with ebusy. We ignore any errors here because this may + // fail on older kernels which don't have + // torvalds/linux@8ed936b5671bfb33d89bc60bdcc7cf0470ba52fe applied. + if err := unix.Rmdir(mountpoint); err != nil && !os.IsNotExist(err) { + logrus.Debugf("Failed to remove %s overlay: %v", id, err) + } return nil }