From cb2dc1cb0b4e0e8837ee0657d5a7a7d013ef5353 Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Mon, 25 Sep 2017 14:42:31 -0700 Subject: [PATCH 1/2] graphdriver/overlay: minor doc comment cleanup Signed-off-by: Euan Kemp Upstream-commit: 1e214c09524c0cf32c3e8005631bbcf3e1afa506 Component: engine --- .../daemon/graphdriver/overlay/overlay.go | 9 ++++--- .../daemon/graphdriver/overlay2/overlay.go | 24 +++++++++++-------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/components/engine/daemon/graphdriver/overlay/overlay.go b/components/engine/daemon/graphdriver/overlay/overlay.go index 853318de59..ff4e300321 100644 --- a/components/engine/daemon/graphdriver/overlay/overlay.go +++ b/components/engine/daemon/graphdriver/overlay/overlay.go @@ -109,8 +109,10 @@ func init() { } // Init returns the NaiveDiffDriver, a native diff driver for overlay filesystem. -// If overlay filesystem is not supported on the host, graphdriver.ErrNotSupported is returned as error. -// If an overlay filesystem is not supported over an existing filesystem then error graphdriver.ErrIncompatibleFS is returned. +// If overlay filesystem is not supported on the host, the error +// graphdriver.ErrNotSupported is returned. +// If an overlay filesystem is not supported over an existing filesystem then +// error graphdriver.ErrIncompatibleFS is returned. func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (graphdriver.Driver, error) { if err := supportsOverlay(); err != nil { @@ -199,7 +201,8 @@ func (d *Driver) Status() [][2]string { } } -// GetMetadata returns meta data about the overlay driver such as root, LowerDir, UpperDir, WorkDir and MergeDir used to store data. +// GetMetadata returns metadata about the overlay driver such as root, +// LowerDir, UpperDir, WorkDir and MergeDir used to store data. func (d *Driver) GetMetadata(id string) (map[string]string, error) { dir := d.dir(id) if _, err := os.Stat(dir); err != nil { diff --git a/components/engine/daemon/graphdriver/overlay2/overlay.go b/components/engine/daemon/graphdriver/overlay2/overlay.go index c2023c7252..9fea3c48b8 100644 --- a/components/engine/daemon/graphdriver/overlay2/overlay.go +++ b/components/engine/daemon/graphdriver/overlay2/overlay.go @@ -77,7 +77,7 @@ const ( maxDepth = 128 // idLength represents the number of random characters - // which can be used to create the unique link identifer + // which can be used to create the unique link identifier // for every layer. If this value is too long then the // page size limit for the mount command may be exceeded. // The idLength should be selected such that following equation @@ -91,7 +91,8 @@ type overlayOptions struct { quota quota.Quota } -// Driver contains information about the home directory and the list of active mounts that are created using this driver. +// Driver contains information about the home directory and the list of active +// mounts that are created using this driver. type Driver struct { home string uidMaps []idtools.IDMap @@ -116,9 +117,11 @@ func init() { graphdriver.Register(driverName, Init) } -// Init returns the a native diff driver for overlay filesystem. -// If overlay filesystem is not supported on the host, graphdriver.ErrNotSupported is returned as error. -// If an overlay filesystem is not supported over an existing filesystem then error graphdriver.ErrIncompatibleFS is returned. +// Init returns the native diff driver for overlay filesystem. +// If overlay filesystem is not supported on the host, the error +// graphdriver.ErrNotSupported is returned. +// If an overlay filesystem is not supported over an existing filesystem then +// the error graphdriver.ErrIncompatibleFS is returned. func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (graphdriver.Driver, error) { opts, err := parseOptions(options) if err != nil { @@ -289,8 +292,8 @@ func (d *Driver) Status() [][2]string { } } -// GetMetadata returns meta data about the overlay driver such as -// LowerDir, UpperDir, WorkDir and MergeDir used to store data. +// GetMetadata returns metadata about the overlay driver such as the LowerDir, +// UpperDir, WorkDir, and MergeDir used to store data. func (d *Driver) GetMetadata(id string) (map[string]string, error) { dir := d.dir(id) if _, err := os.Stat(dir); err != nil { @@ -636,7 +639,8 @@ func (d *Driver) Exists(id string) bool { return err == nil } -// isParent returns if the passed in parent is the direct parent of the passed in layer +// isParent determines whether the given parent is the direct parent of the +// given layer id func (d *Driver) isParent(id, parent string) bool { lowers, err := d.getLowerDirs(id) if err != nil { @@ -711,8 +715,8 @@ func (d *Driver) Diff(id, parent string) (io.ReadCloser, error) { }) } -// Changes produces a list of changes between the specified layer -// and its parent layer. If parent is "", then all changes will be ADD changes. +// Changes produces a list of changes between the specified layer and its +// parent layer. If parent is "", then all changes will be ADD changes. func (d *Driver) Changes(id, parent string) ([]archive.Change, error) { if useNaiveDiff(d.home) || !d.isParent(id, parent) { return d.naiveDiff.Changes(id, parent) From 7830091c4bc65fef4a3e3c9aaf8ac475b83767dc Mon Sep 17 00:00:00 2001 From: Euan Kemp Date: Mon, 20 Nov 2017 21:33:11 -0800 Subject: [PATCH 2/2] 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 }