On Sun, Apr 02, 2017 at 05:58:41PM -0700, Linus Torvalds wrote:
> I had to go and double-check that "DCACHE_DIRECTORY_TYPE" is what
> d_can_lookup() actually checks, so _that_ part is perhaps a bit
> subtle, and might be worth noting in that comment that you edited.
>
> So the real "rule" ends up being that we only ever look up things from
> dentries of type DCACHE_DIRECTORY_TYPE set, and those had better have
> DCACHE_RCUACCESS bit set.
>
> And the only reason path_init() only checks it for that case is that
> nd->root and nd->pwd both have to be of type d_can_lookup().
>
> Do we check that when we set it? I hope/assume we do.
For chdir()/chroot()/pivot_root() it's done by LOOKUP_DIRECTORY in lookup
flags; fchdir() is slightly different - there we check S_ISDIR of inode
of opened file. Which is almost the same thing, except for
kinda-sorta directories that have no ->lookup() - we have them for
NFS referral points. It should be impossible to end up with
one of those opened - not even with O_PATH; follow_managed() will be called
and we'll either fail or cross into whatever ends up overmounting them.
Still, it might be cleaner to turn that check into
d_can_lookup(f.file->f_path.dentry)
simply for consistency sake.
The thing I really don't like is mntns_install(). With sufficiently
nasty nfsroot setup it might be possible to end up with referral point
as one's root/pwd; getting out of such state would be interesting...
Smells like that place should be a solitary follow_down(), not a loop
of follow_down_one(), but I want Eric's opinion on that one; userns stuff
is weird.
diff --git a/fs/dcache.c b/fs/dcache.c
index 95d71eda8142..05550139a8a6 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1757,7 +1757,13 @@ static unsigned d_flags_for_inode(struct inode *inode)
return DCACHE_MISS_TYPE;
if (S_ISDIR(inode->i_mode)) {
- add_flags = DCACHE_DIRECTORY_TYPE;
+ /*
+ * Any potential starting point of lookup should have
+ * DCACHE_RCUACCESS; currently directory dentries
+ * come from d_alloc() anyway, but it costs us nothing
+ * to enforce it here.
+ */
+ add_flags = DCACHE_DIRECTORY_TYPE | DCACHE_RCUACCESS;
if (unlikely(!(inode->i_opflags & IOP_LOOKUP))) {
if (unlikely(!inode->i_op->lookup))
add_flags = DCACHE_AUTODIR_TYPE;
diff --git a/fs/namei.c b/fs/namei.c
index d41fab78798b..19dcf62133cc 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2145,6 +2145,9 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
int retval = 0;
const char *s = nd->name->name;
+ if (!*s)
+ flags &= ~LOOKUP_RCU;
+
nd->last_type = LAST_ROOT; /* if there are only slashes... */
nd->flags = flags | LOOKUP_JUMPED | LOOKUP_PARENT;
nd->depth = 0;
diff --git a/fs/namespace.c b/fs/namespace.c
index cc1375eff88c..31ec9a79d2d4 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3467,6 +3467,7 @@ static int mntns_install(struct nsproxy *nsproxy, struct ns_common *ns)
struct fs_struct *fs = current->fs;
struct mnt_namespace *mnt_ns = to_mnt_ns(ns);
struct path root;
+ int err;
if (!ns_capable(mnt_ns->user_ns, CAP_SYS_ADMIN) ||
!ns_capable(current_user_ns(), CAP_SYS_CHROOT) ||
@@ -3484,15 +3485,14 @@ static int mntns_install(struct nsproxy *nsproxy, struct ns_common *ns)
root.mnt = &mnt_ns->root->mnt;
root.dentry = mnt_ns->root->mnt.mnt_root;
path_get(&root);
- while(d_mountpoint(root.dentry) && follow_down_one(&root))
- ;
-
- /* Update the pwd and root */
- set_fs_pwd(fs, &root);
- set_fs_root(fs, &root);
-
+ err = follow_down(&root);
+ if (likely(!err)) {
+ /* Update the pwd and root */
+ set_fs_pwd(fs, &root);
+ set_fs_root(fs, &root);
+ }
path_put(&root);
- return 0;
+ return err;
}
static struct user_namespace *mntns_owner(struct ns_common *ns)
diff --git a/fs/open.c b/fs/open.c
index 949cef29c3bb..217b5db588c8 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -459,20 +459,17 @@ SYSCALL_DEFINE1(chdir, const char __user *, filename)
SYSCALL_DEFINE1(fchdir, unsigned int, fd)
{
struct fd f = fdget_raw(fd);
- struct inode *inode;
int error = -EBADF;
error = -EBADF;
if (!f.file)
goto out;
- inode = file_inode(f.file);
-
error = -ENOTDIR;
- if (!S_ISDIR(inode->i_mode))
+ if (!d_can_lookup(f.file->f_path.dentry))
goto out_putf;
- error = inode_permission(inode, MAY_EXEC | MAY_CHDIR);
+ error = inode_permission(file_inode(f.file), MAY_EXEC | MAY_CHDIR);
if (!error)
set_fs_pwd(current->fs, &f.file->f_path);
out_putf:
