Commit 85fdee1e authored by Amir Goldstein's avatar Amir Goldstein Committed by Miklos Szeredi

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: 2cac0c00 ("ovl: get exclusive ownership on upper/work dirs")
Cc: <stable@vger.kernel.org> # v4.13
Reported-by: default avatarEuan Kemp <euank@euank.com>
Reported-by: default avatarVivek Goyal <vgoyal@redhat.com>
Signed-off-by: default avatarAmir Goldstein <amir73il@gmail.com>
Signed-off-by: default avatarMiklos Szeredi <mszeredi@redhat.com>
parent 5820dc08
...@@ -210,8 +210,11 @@ path as another overlay mount and it may use a lower layer path that is ...@@ -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
......
...@@ -37,6 +37,9 @@ struct ovl_fs { ...@@ -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 */
......
...@@ -211,9 +211,10 @@ static void ovl_put_super(struct super_block *sb) ...@@ -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);
if (ufs->workdir_locked)
ovl_inuse_unlock(ufs->workbasedir); 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) ...@@ -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) ...@@ -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,10 +1165,12 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) ...@@ -1156,10 +1165,12 @@ 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:
if (ufs->workdir_locked)
ovl_inuse_unlock(workpath.dentry); ovl_inuse_unlock(workpath.dentry);
out_put_workpath: out_put_workpath:
path_put(&workpath); path_put(&workpath);
out_unlock_upperdentry: out_unlock_upperdentry:
if (ufs->upperdir_locked)
ovl_inuse_unlock(upperpath.dentry); ovl_inuse_unlock(upperpath.dentry);
out_put_upperpath: out_put_upperpath:
path_put(&upperpath); path_put(&upperpath);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment