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: 31+ 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 14:24 ` Greg Kroah-Hartman
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).