4.13-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Amir Goldstein <amir7...@gmail.com>

commit 85fdee1eef1a9e48ad5716916677e0c5fbc781e3 upstream.

Enforcing exclusive ownership on upper/work dirs caused a docker
regression: https://github.com/moby/moby/issues/34672.

Euan spotted the regression and pointed to the offending commit.
Vivek has brought the regression to my attention and provided this
reproducer:

Terminal 1:

  mount -t overlay -o workdir=work,lowerdir=lower,upperdir=upper none
        merged/

Terminal 2:

  unshare -m

Terminal 1:

  umount merged
  mount -t overlay -o workdir=work,lowerdir=lower,upperdir=upper none
        merged/
  mount: /root/overlay-testing/merged: none already mounted or mount point
         busy

To fix the regression, I replaced the error with an alarming warning.
With index feature enabled, mount does fail, but logs a suggestion to
override exclusive dir protection by disabling index.
Note that index=off mount does take the inuse locks, so a concurrent
index=off will issue the warning and a concurrent index=on mount will fail.

Documentation was updated to reflect this change.

Fixes: 2cac0c00a6cd ("ovl: get exclusive ownership on upper/work dirs")
Reported-by: Euan Kemp <eu...@euank.com>
Reported-by: Vivek Goyal <vgo...@redhat.com>
Signed-off-by: Amir Goldstein <amir7...@gmail.com>
Signed-off-by: Miklos Szeredi <mszer...@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>

---
 Documentation/filesystems/overlayfs.txt |    5 ++++-
 fs/overlayfs/ovl_entry.h                |    3 +++
 fs/overlayfs/super.c                    |   27 +++++++++++++++++++--------
 3 files changed, 26 insertions(+), 9 deletions(-)

--- a/Documentation/filesystems/overlayfs.txt
+++ b/Documentation/filesystems/overlayfs.txt
@@ -210,8 +210,11 @@ path as another overlay mount and it may
 beneath or above the path of another overlay lower layer path.
 
 Using an upper layer path and/or a workdir path that are already used by
-another overlay mount is not allowed and will fail with EBUSY.  Using
+another overlay mount is not allowed and may fail with EBUSY.  Using
 partially overlapping paths is not allowed but will not fail with EBUSY.
+If files are accessed from two overlayfs mounts which share or overlap the
+upper layer and/or workdir path the behavior of the overlay is undefined,
+though it will not result in a crash or deadlock.
 
 Mounting an overlay using an upper layer path, where the upper layer path
 was previously used by another mounted overlay in combination with a
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -37,6 +37,9 @@ struct ovl_fs {
        bool noxattr;
        /* sb common to all layers */
        struct super_block *same_sb;
+       /* Did we take the inuse lock? */
+       bool upperdir_locked;
+       bool workdir_locked;
 };
 
 /* private information held for every overlayfs dentry */
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -210,9 +210,10 @@ static void ovl_put_super(struct super_b
 
        dput(ufs->indexdir);
        dput(ufs->workdir);
-       ovl_inuse_unlock(ufs->workbasedir);
+       if (ufs->workdir_locked)
+               ovl_inuse_unlock(ufs->workbasedir);
        dput(ufs->workbasedir);
-       if (ufs->upper_mnt)
+       if (ufs->upper_mnt && ufs->upperdir_locked)
                ovl_inuse_unlock(ufs->upper_mnt->mnt_root);
        mntput(ufs->upper_mnt);
        for (i = 0; i < ufs->numlower; i++)
@@ -880,9 +881,13 @@ static int ovl_fill_super(struct super_b
                        goto out_put_upperpath;
 
                err = -EBUSY;
-               if (!ovl_inuse_trylock(upperpath.dentry)) {
-                       pr_err("overlayfs: upperdir is in-use by another 
mount\n");
+               if (ovl_inuse_trylock(upperpath.dentry)) {
+                       ufs->upperdir_locked = true;
+               } else if (ufs->config.index) {
+                       pr_err("overlayfs: upperdir is in-use by another mount, 
mount with '-o index=off' to override exclusive upperdir protection.\n");
                        goto out_put_upperpath;
+               } else {
+                       pr_warn("overlayfs: upperdir is in-use by another 
mount, accessing files from both mounts will result in undefined behavior.\n");
                }
 
                err = ovl_mount_dir(ufs->config.workdir, &workpath);
@@ -900,9 +905,13 @@ static int ovl_fill_super(struct super_b
                }
 
                err = -EBUSY;
-               if (!ovl_inuse_trylock(workpath.dentry)) {
-                       pr_err("overlayfs: workdir is in-use by another 
mount\n");
+               if (ovl_inuse_trylock(workpath.dentry)) {
+                       ufs->workdir_locked = true;
+               } else if (ufs->config.index) {
+                       pr_err("overlayfs: workdir is in-use by another mount, 
mount with '-o index=off' to override exclusive workdir protection.\n");
                        goto out_put_workpath;
+               } else {
+                       pr_warn("overlayfs: workdir is in-use by another mount, 
accessing files from both mounts will result in undefined behavior.\n");
                }
 
                ufs->workbasedir = workpath.dentry;
@@ -1155,11 +1164,13 @@ out_put_lowerpath:
 out_free_lowertmp:
        kfree(lowertmp);
 out_unlock_workdentry:
-       ovl_inuse_unlock(workpath.dentry);
+       if (ufs->workdir_locked)
+               ovl_inuse_unlock(workpath.dentry);
 out_put_workpath:
        path_put(&workpath);
 out_unlock_upperdentry:
-       ovl_inuse_unlock(upperpath.dentry);
+       if (ufs->upperdir_locked)
+               ovl_inuse_unlock(upperpath.dentry);
 out_put_upperpath:
        path_put(&upperpath);
 out_free_config:


Reply via email to