All of lore.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: John Ogness <john.ogness@linutronix.de>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Christoph Hellwig <hch@lst.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: [PATCH] fs: Teach path_connected to handle nfs filesystems with multiple roots.
Date: Wed, 14 Mar 2018 18:20:29 -0500	[thread overview]
Message-ID: <87muzai6rm.fsf_-_@xmission.com> (raw)
In-Reply-To: <20180313005010.GV30522@ZenIV.linux.org.uk> (Al Viro's message of "Tue, 13 Mar 2018 00:50:11 +0000")


On nfsv2 and nfsv3 the nfs server can export subsets of the same
filesystem and report the same filesystem identifier, so that the nfs
client can know they are the same filesystem.  The subsets can be from
disjoint directory trees.  The nfsv2 and nfsv3 filesystems provides no
way to find the common root of all directory trees exported form the
server with the same filesystem identifier.

The practical result is that in struct super s_root for nfs s_root is
not necessarily the root of the filesystem.  The nfs mount code sets
s_root to the root of the first subset of the nfs filesystem that the
kernel mounts.

This effects the dcache invalidation code in generic_shutdown_super
currently called shrunk_dcache_for_umount and that code for years
has gone through an additional list of dentries that might be dentry
trees that need to be freed to accomodate nfs.

When I wrote path_connected I did not realize nfs was so special, and
it's hueristic for avoiding calling is_subdir can fail.

The practical case where this fails is when there is a move of a
directory from the subtree exposed by one nfs mount to the subtree
exposed by another nfs mount.  This move can happen either locally or
remotely.  With the remote case requiring that the move directory be cached
before the move and that after the move someone walks the path
to where the move directory now exists and in so doing causes the
already cached directory to be moved in the dcache through the magic
of d_splice_alias.

If someone whose working directory is in the move directory or a
subdirectory and now starts calling .. from the initial mount of nfs
(where s_root == mnt_root), then path_connected as a heuristic will
not bother with the is_subdir check.  As s_root really is not the root
of the nfs filesystem this heuristic is wrong, and the path may
actually not be connected and path_connected can fail.

The is_subdir function might be cheap enough that we can call it
unconditionally.  Verifying that will take some benchmarking and
the result may not be the same on all kernels this fix needs
to be backported to.  So I am avoiding that for now.

Filesystems with snapshots such as nilfs and btrfs do something
similar.  But as the directory tree of the snapshots are disjoint
from one another and from the main directory tree rename won't move
things between them and this problem will not occur.

Cc: stable@vger.kernel.org
Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Fixes: 397d425dc26d ("vfs: Test for and handle paths that are unreachable from their mnt_root")
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---

Al do you want to push this one to Linus or shall I?

 fs/namei.c         | 5 +++--
 fs/nfs/super.c     | 2 ++
 include/linux/fs.h | 1 +
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 921ae32dbc80..cafa365eeb70 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -559,9 +559,10 @@ static int __nd_alloc_stack(struct nameidata *nd)
 static bool path_connected(const struct path *path)
 {
 	struct vfsmount *mnt = path->mnt;
+	struct super_block *sb = mnt->mnt_sb;
 
-	/* Only bind mounts can have disconnected paths */
-	if (mnt->mnt_root == mnt->mnt_sb->s_root)
+	/* Bind mounts and multi-root filesystems can have disconnected paths */
+	if (!(sb->s_iflags & SB_I_MULTIROOT) && (mnt->mnt_root == sb->s_root))
 		return true;
 
 	return is_subdir(path->dentry, mnt->mnt_root);
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 29bacdc56f6a..5e470e233c83 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2631,6 +2631,8 @@ struct dentry *nfs_fs_mount_common(struct nfs_server *server,
 		/* initial superblock/root creation */
 		mount_info->fill_super(s, mount_info);
 		nfs_get_cache_cookie(s, mount_info->parsed, mount_info->cloned);
+		if (!(server->flags & NFS_MOUNT_UNSHARED))
+			s->s_iflags |= SB_I_MULTIROOT;
 	}
 
 	mntroot = nfs_get_root(s, mount_info->mntfh, dev_name);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2a815560fda0..0430e03febaa 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1317,6 +1317,7 @@ extern int send_sigurg(struct fown_struct *fown);
 #define SB_I_CGROUPWB	0x00000001	/* cgroup-aware writeback enabled */
 #define SB_I_NOEXEC	0x00000002	/* Ignore executables on this fs */
 #define SB_I_NODEV	0x00000004	/* Ignore devices on this fs */
+#define SB_I_MULTIROOT	0x00000008	/* Multiple roots to the dentry tree */
 
 /* sb->s_iflags to limit user namespace mounts */
 #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
-- 
2.14.1

  parent reply	other threads:[~2018-03-14 23:21 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-22 23:50 [PATCH v2 0/6] fs/dcache: avoid trylock loops John Ogness
2018-02-22 23:50 ` [PATCH v2 1/6] fs/dcache: Remove stale comment from dentry_kill() John Ogness
2018-02-22 23:50 ` [PATCH v2 2/6] fs/dcache: Move dentry_kill() below lock_parent() John Ogness
2018-02-22 23:50 ` [PATCH v2 3/6] fs/dcache: Avoid the try_lock loop in d_delete() John Ogness
2018-02-23  2:08   ` Al Viro
2018-02-22 23:50 ` [PATCH v2 4/6] fs/dcache: Avoid the try_lock loops in dentry_kill() John Ogness
2018-02-23  2:22   ` Al Viro
2018-02-23  3:12     ` Al Viro
2018-02-23  3:16       ` Al Viro
2018-02-23  5:46       ` Al Viro
2018-02-22 23:50 ` [PATCH v2 5/6] fs/dcache: Avoid a try_lock loop in shrink_dentry_list() John Ogness
2018-02-23  3:48   ` Al Viro
2018-02-22 23:50 ` [PATCH v2 6/6] fs/dcache: Avoid remaining " John Ogness
2018-02-23  3:58   ` Al Viro
2018-02-23  4:08     ` Al Viro
2018-02-23 13:57       ` John Ogness
2018-02-23 15:09         ` Al Viro
2018-02-23 17:42           ` Al Viro
2018-02-23 20:13             ` [BUG] lock_parent() breakage when used from shrink_dentry_list() (was Re: [PATCH v2 6/6] fs/dcache: Avoid remaining try_lock loop in shrink_dentry_list()) Al Viro
2018-02-23 21:35               ` Linus Torvalds
2018-02-24  0:22                 ` Al Viro
2018-02-25  7:40                   ` Al Viro
2018-02-27  5:16                     ` dcache: remove trylock loops (was Re: [BUG] lock_parent() breakage when used from shrink_dentry_list()) John Ogness
2018-03-12 19:13                       ` Al Viro
2018-03-12 20:05                         ` Al Viro
2018-03-12 20:33                           ` Al Viro
2018-03-13  1:12                           ` NeilBrown
2018-04-28  0:10                             ` Al Viro
2018-03-12 20:23                         ` Eric W. Biederman
2018-03-12 20:39                           ` Al Viro
2018-03-12 23:28                             ` Eric W. Biederman
2018-03-12 23:52                               ` Eric W. Biederman
2018-03-13  0:37                                 ` Al Viro
2018-03-13  0:50                                   ` Al Viro
2018-03-13  4:02                                     ` Eric W. Biederman
2018-03-14 23:20                                     ` Eric W. Biederman [this message]
2018-03-15 22:34                                       ` [PATCH] fs: Teach path_connected to handle nfs filesystems with multiple roots Al Viro
2018-03-13  0:36                               ` dcache: remove trylock loops (was Re: [BUG] lock_parent() breakage when used from shrink_dentry_list()) Al Viro
2018-03-12 22:14                         ` Thomas Gleixner
2018-03-13 20:46                         ` John Ogness
2018-03-13 21:05                           ` John Ogness
2018-03-13 23:59                             ` Al Viro
2018-03-14  2:58                               ` Matthew Wilcox
2018-03-14  8:18                               ` John Ogness
2018-03-02  9:04                     ` [BUG] lock_parent() breakage when used from shrink_dentry_list() (was Re: [PATCH v2 6/6] fs/dcache: Avoid remaining try_lock loop in shrink_dentry_list()) Sebastian Andrzej Siewior
2018-02-23  0:59 ` [PATCH v2 0/6] fs/dcache: avoid trylock loops Linus Torvalds

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=87muzai6rm.fsf_-_@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=bigeasy@linutronix.de \
    --cc=hch@lst.de \
    --cc=john.ogness@linutronix.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ZenIV.linux.org.uk \
    /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.