ovl: fix regression caused by exclusive upper/work dir protection

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: 2cac0c00a6 ("ovl: get exclusive ownership on upper/work dirs")
Cc: <stable@vger.kernel.org> # v4.13
Reported-by: Euan Kemp <euank@euank.com>
Reported-by: Vivek Goyal <vgoyal@redhat.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
This commit is contained in:
Amir Goldstein 2017-09-29 10:21:21 +03:00 committed by Miklos Szeredi
parent 5820dc0888
commit 85fdee1eef
3 changed files with 26 additions and 9 deletions

View file

@ -210,8 +210,11 @@ path as another overlay mount and it may use a lower layer path that is
beneath or above the path of another overlay lower layer path. 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 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. 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 Mounting an overlay using an upper layer path, where the upper layer path
was previously used by another mounted overlay in combination with a was previously used by another mounted overlay in combination with a

View file

@ -37,6 +37,9 @@ struct ovl_fs {
bool noxattr; bool noxattr;
/* sb common to all layers */ /* sb common to all layers */
struct super_block *same_sb; struct super_block *same_sb;
/* Did we take the inuse lock? */
bool upperdir_locked;
bool workdir_locked;
}; };
/* private information held for every overlayfs dentry */ /* private information held for every overlayfs dentry */

View file

@ -211,9 +211,10 @@ static void ovl_put_super(struct super_block *sb)
dput(ufs->indexdir); dput(ufs->indexdir);
dput(ufs->workdir); dput(ufs->workdir);
ovl_inuse_unlock(ufs->workbasedir); if (ufs->workdir_locked)
ovl_inuse_unlock(ufs->workbasedir);
dput(ufs->workbasedir); dput(ufs->workbasedir);
if (ufs->upper_mnt) if (ufs->upper_mnt && ufs->upperdir_locked)
ovl_inuse_unlock(ufs->upper_mnt->mnt_root); ovl_inuse_unlock(ufs->upper_mnt->mnt_root);
mntput(ufs->upper_mnt); mntput(ufs->upper_mnt);
for (i = 0; i < ufs->numlower; i++) for (i = 0; i < ufs->numlower; i++)
@ -881,9 +882,13 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
goto out_put_upperpath; goto out_put_upperpath;
err = -EBUSY; err = -EBUSY;
if (!ovl_inuse_trylock(upperpath.dentry)) { if (ovl_inuse_trylock(upperpath.dentry)) {
pr_err("overlayfs: upperdir is in-use by another mount\n"); 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; 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); err = ovl_mount_dir(ufs->config.workdir, &workpath);
@ -901,9 +906,13 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
} }
err = -EBUSY; err = -EBUSY;
if (!ovl_inuse_trylock(workpath.dentry)) { if (ovl_inuse_trylock(workpath.dentry)) {
pr_err("overlayfs: workdir is in-use by another mount\n"); 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; 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; ufs->workbasedir = workpath.dentry;
@ -1156,11 +1165,13 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
out_free_lowertmp: out_free_lowertmp:
kfree(lowertmp); kfree(lowertmp);
out_unlock_workdentry: out_unlock_workdentry:
ovl_inuse_unlock(workpath.dentry); if (ufs->workdir_locked)
ovl_inuse_unlock(workpath.dentry);
out_put_workpath: out_put_workpath:
path_put(&workpath); path_put(&workpath);
out_unlock_upperdentry: out_unlock_upperdentry:
ovl_inuse_unlock(upperpath.dentry); if (ufs->upperdir_locked)
ovl_inuse_unlock(upperpath.dentry);
out_put_upperpath: out_put_upperpath:
path_put(&upperpath); path_put(&upperpath);
out_free_config: out_free_config: