From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, Amir Goldstein <amir73il@gmail.com>,
Colin Walters <walters@verbum.org>,
Miklos Szeredi <mszeredi@redhat.com>
Subject: [PATCH 5.3 18/21] ovl: fix regression caused by overlapping layers detection
Date: Fri, 20 Sep 2019 00:03:19 +0200 [thread overview]
Message-ID: <20190919214711.175921151@linuxfoundation.org> (raw)
In-Reply-To: <20190919214657.842130855@linuxfoundation.org>
From: Amir Goldstein <amir73il@gmail.com>
commit 0be0bfd2de9dfdd2098a9c5b14bdd8f739c9165d upstream.
Once upon a time, commit 2cac0c00a6cd ("ovl: get exclusive ownership on
upper/work dirs") in v4.13 added some sanity checks on overlayfs layers.
This change caused a docker regression. The root cause was mount leaks
by docker, which as far as I know, still exist.
To mitigate the regression, commit 85fdee1eef1a ("ovl: fix regression
caused by exclusive upper/work dir protection") in v4.14 turned the
mount errors into warnings for the default index=off configuration.
Recently, commit 146d62e5a586 ("ovl: detect overlapping layers") in
v5.2, re-introduced exclusive upper/work dir checks regardless of
index=off configuration.
This changes the status quo and mount leak related bug reports have
started to re-surface. Restore the status quo to fix the regressions.
To clarify, index=off does NOT relax overlapping layers check for this
ovelayfs mount. index=off only relaxes exclusive upper/work dir checks
with another overlayfs mount.
To cover the part of overlapping layers detection that used the
exclusive upper/work dir checks to detect overlap with self upper/work
dir, add a trap also on the work base dir.
Link: https://github.com/moby/moby/issues/34672
Link: https://lore.kernel.org/linux-fsdevel/20171006121405.GA32700@veci.piliscsaba.szeredi.hu/
Link: https://github.com/containers/libpod/issues/3540
Fixes: 146d62e5a586 ("ovl: detect overlapping layers")
Cc: <stable@vger.kernel.org> # v4.19+
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Tested-by: Colin Walters <walters@verbum.org>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Documentation/filesystems/overlayfs.txt | 2
fs/overlayfs/ovl_entry.h | 1
fs/overlayfs/super.c | 73 ++++++++++++++++++++------------
3 files changed, 49 insertions(+), 27 deletions(-)
--- a/Documentation/filesystems/overlayfs.txt
+++ b/Documentation/filesystems/overlayfs.txt
@@ -302,7 +302,7 @@ beneath or above the path of another ove
Using an upper layer path and/or a workdir path that are already used by
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 and may 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.
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -66,6 +66,7 @@ struct ovl_fs {
bool workdir_locked;
/* Traps in ovl inode cache */
struct inode *upperdir_trap;
+ struct inode *workbasedir_trap;
struct inode *workdir_trap;
struct inode *indexdir_trap;
/* Inode numbers in all layers do not use the high xino_bits */
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -212,6 +212,7 @@ static void ovl_free_fs(struct ovl_fs *o
{
unsigned i;
+ iput(ofs->workbasedir_trap);
iput(ofs->indexdir_trap);
iput(ofs->workdir_trap);
iput(ofs->upperdir_trap);
@@ -1003,6 +1004,25 @@ static int ovl_setup_trap(struct super_b
return 0;
}
+/*
+ * Determine how we treat concurrent use of upperdir/workdir based on the
+ * index feature. This is papering over mount leaks of container runtimes,
+ * for example, an old overlay mount is leaked and now its upperdir is
+ * attempted to be used as a lower layer in a new overlay mount.
+ */
+static int ovl_report_in_use(struct ovl_fs *ofs, const char *name)
+{
+ if (ofs->config.index) {
+ pr_err("overlayfs: %s is in-use as upperdir/workdir of another mount, mount with '-o index=off' to override exclusive upperdir protection.\n",
+ name);
+ return -EBUSY;
+ } else {
+ pr_warn("overlayfs: %s is in-use as upperdir/workdir of another mount, accessing files from both mounts will result in undefined behavior.\n",
+ name);
+ return 0;
+ }
+}
+
static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
struct path *upperpath)
{
@@ -1040,14 +1060,12 @@ static int ovl_get_upper(struct super_bl
upper_mnt->mnt_flags &= ~(MNT_NOATIME | MNT_NODIRATIME | MNT_RELATIME);
ofs->upper_mnt = upper_mnt;
- err = -EBUSY;
if (ovl_inuse_trylock(ofs->upper_mnt->mnt_root)) {
ofs->upperdir_locked = true;
- } else if (ofs->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;
} else {
- pr_warn("overlayfs: upperdir is in-use by another mount, accessing files from both mounts will result in undefined behavior.\n");
+ err = ovl_report_in_use(ofs, "upperdir");
+ if (err)
+ goto out;
}
err = 0;
@@ -1157,16 +1175,19 @@ static int ovl_get_workdir(struct super_
ofs->workbasedir = dget(workpath.dentry);
- err = -EBUSY;
if (ovl_inuse_trylock(ofs->workbasedir)) {
ofs->workdir_locked = true;
- } else if (ofs->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;
} else {
- pr_warn("overlayfs: workdir is in-use by another mount, accessing files from both mounts will result in undefined behavior.\n");
+ err = ovl_report_in_use(ofs, "workdir");
+ if (err)
+ goto out;
}
+ err = ovl_setup_trap(sb, ofs->workbasedir, &ofs->workbasedir_trap,
+ "workdir");
+ if (err)
+ goto out;
+
err = ovl_make_workdir(sb, ofs, &workpath);
out:
@@ -1313,16 +1334,16 @@ static int ovl_get_lower_layers(struct s
if (err < 0)
goto out;
- err = -EBUSY;
- if (ovl_is_inuse(stack[i].dentry)) {
- pr_err("overlayfs: lowerdir is in-use as upperdir/workdir\n");
- goto out;
- }
-
err = ovl_setup_trap(sb, stack[i].dentry, &trap, "lowerdir");
if (err)
goto out;
+ if (ovl_is_inuse(stack[i].dentry)) {
+ err = ovl_report_in_use(ofs, "lowerdir");
+ if (err)
+ goto out;
+ }
+
mnt = clone_private_mount(&stack[i]);
err = PTR_ERR(mnt);
if (IS_ERR(mnt)) {
@@ -1469,8 +1490,8 @@ out_err:
* - another layer of this overlayfs instance
* - upper/work dir of any overlayfs instance
*/
-static int ovl_check_layer(struct super_block *sb, struct dentry *dentry,
- const char *name)
+static int ovl_check_layer(struct super_block *sb, struct ovl_fs *ofs,
+ struct dentry *dentry, const char *name)
{
struct dentry *next = dentry, *parent;
int err = 0;
@@ -1482,13 +1503,11 @@ static int ovl_check_layer(struct super_
/* Walk back ancestors to root (inclusive) looking for traps */
while (!err && parent != next) {
- if (ovl_is_inuse(parent)) {
- err = -EBUSY;
- pr_err("overlayfs: %s path overlapping in-use upperdir/workdir\n",
- name);
- } else if (ovl_lookup_trap_inode(sb, parent)) {
+ if (ovl_lookup_trap_inode(sb, parent)) {
err = -ELOOP;
pr_err("overlayfs: overlapping %s path\n", name);
+ } else if (ovl_is_inuse(parent)) {
+ err = ovl_report_in_use(ofs, name);
}
next = parent;
parent = dget_parent(next);
@@ -1509,7 +1528,8 @@ static int ovl_check_overlapping_layers(
int i, err;
if (ofs->upper_mnt) {
- err = ovl_check_layer(sb, ofs->upper_mnt->mnt_root, "upperdir");
+ err = ovl_check_layer(sb, ofs, ofs->upper_mnt->mnt_root,
+ "upperdir");
if (err)
return err;
@@ -1520,13 +1540,14 @@ static int ovl_check_overlapping_layers(
* workbasedir. In that case, we already have their traps in
* inode cache and we will catch that case on lookup.
*/
- err = ovl_check_layer(sb, ofs->workbasedir, "workdir");
+ err = ovl_check_layer(sb, ofs, ofs->workbasedir, "workdir");
if (err)
return err;
}
for (i = 0; i < ofs->numlower; i++) {
- err = ovl_check_layer(sb, ofs->lower_layers[i].mnt->mnt_root,
+ err = ovl_check_layer(sb, ofs,
+ ofs->lower_layers[i].mnt->mnt_root,
"lowerdir");
if (err)
return err;
next prev parent reply other threads:[~2019-09-19 22:05 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-19 22:03 [PATCH 5.3 00/21] 5.3.1-stable review Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 01/21] USB: usbcore: Fix slab-out-of-bounds bug during device reset Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 02/21] media: tm6000: double free if usb disconnect while streaming Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 03/21] phy: renesas: rcar-gen3-usb2: Disable clearing VBUS in over-current Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 04/21] ip6_gre: fix a dst leak in ip6erspan_tunnel_xmit Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 05/21] net/sched: fix race between deactivation and dequeue for NOLOCK qdisc Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 06/21] net_sched: let qdisc_put() accept NULL pointer Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 07/21] udp: correct reuseport selection with connected sockets Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 08/21] xen-netfront: do not assume sk_buff_head list is empty in error handling Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 09/21] net: dsa: Fix load order between DSA drivers and taggers Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 10/21] net: stmmac: Hold rtnl lock in suspend/resume callbacks Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 11/21] KVM: coalesced_mmio: add bounds checking Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 12/21] Documentation: sphinx: Add missing comma to list of strings Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 13/21] firmware: google: check if size is valid when decoding VPD data Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 14/21] serial: sprd: correct the wrong sequence of arguments Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 15/21] tty/serial: atmel: reschedule TX after RX was started Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 16/21] nl80211: Fix possible Spectre-v1 for CQM RSSI thresholds Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 17/21] Revert "arm64: Remove unnecessary ISBs from set_{pte,pmd,pud}" Greg Kroah-Hartman
2019-09-19 22:03 ` Greg Kroah-Hartman [this message]
2019-09-19 22:03 ` [PATCH 5.3 19/21] phy: qcom-qmp: Correct ready status, again Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 20/21] floppy: fix usercopy direction Greg Kroah-Hartman
2019-09-19 22:03 ` [PATCH 5.3 21/21] media: technisat-usb2: break out of loop at end of buffer Greg Kroah-Hartman
2019-09-20 13:45 ` [PATCH 5.3 00/21] 5.3.1-stable review Guenter Roeck
2019-09-20 13:54 ` Jon Hunter
2019-09-20 13:54 ` Jon Hunter
2019-09-20 14:24 ` Greg Kroah-Hartman
2019-09-20 16:01 ` Jon Hunter
2019-09-20 16:01 ` Jon Hunter
2019-09-22 8:13 ` Greg Kroah-Hartman
2019-09-20 14:41 ` Naresh Kamboju
2019-09-21 5:06 ` Greg Kroah-Hartman
2019-09-20 21:17 ` shuah
2019-09-21 5:04 ` Greg Kroah-Hartman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190919214711.175921151@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=amir73il@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mszeredi@redhat.com \
--cc=stable@vger.kernel.org \
--cc=walters@verbum.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.