nfs: fix ->d_revalidate() UAF on ->d_name accesses

Pass the stable name all the way down to ->rpc_ops->lookup() instances.

Note that passing &dentry->d_name is safe in e.g. nfs_lookup() - it *is*
stable there, as it is in ->create() et.al.

dget_parent() in nfs_instantiate() should be redundant - it'd better be
stable there; if it's not, we have more trouble, since ->d_name would
also be unsafe in such case.

nfs_submount() and nfs4_submount() may or may not require fixes - if
they ever get moved on server with fhandle preserved, we are in trouble
there...

UAF window is fairly narrow here and exfiltration requires the ability
to watch the traffic.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index c28983ee75caaf67ca2990e120eb40335871f00a..2b04038b0e40525be67f1ec512fb0db547e5f6ed 100644 (file)
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1672,7 +1672,7 @@ nfs_lookup_revalidate_delegated(struct inode *dir, struct dentry *dentry,
        return nfs_lookup_revalidate_done(dir, dentry, inode, 1);
 }
 
-static int nfs_lookup_revalidate_dentry(struct inode *dir,
+static int nfs_lookup_revalidate_dentry(struct inode *dir, const struct qstr *name,
                                        struct dentry *dentry,
                                        struct inode *inode, unsigned int flags)
 {
@@ -1690,7 +1690,7 @@ static int nfs_lookup_revalidate_dentry(struct inode *dir,
                goto out;
 
        dir_verifier = nfs_save_change_attribute(dir);
-       ret = NFS_PROTO(dir)->lookup(dir, dentry, fhandle, fattr);
+       ret = NFS_PROTO(dir)->lookup(dir, dentry, name, fhandle, fattr);
        if (ret < 0)
                goto out;
 
@@ -1775,7 +1775,7 @@ nfs_do_lookup_revalidate(struct inode *dir, const struct qstr *name,
        if (NFS_STALE(inode))
                goto out_bad;
 
-       return nfs_lookup_revalidate_dentry(dir, dentry, inode, flags);
+       return nfs_lookup_revalidate_dentry(dir, name, dentry, inode, flags);
 out_valid:
        return nfs_lookup_revalidate_done(dir, dentry, inode, 1);
 out_bad:
@@ -1970,7 +1970,8 @@ struct dentry *nfs_lookup(struct inode *dir, struct dentry * dentry, unsigned in
 
        dir_verifier = nfs_save_change_attribute(dir);
        trace_nfs_lookup_enter(dir, dentry, flags);
-       error = NFS_PROTO(dir)->lookup(dir, dentry, fhandle, fattr);
+       error = NFS_PROTO(dir)->lookup(dir, dentry, &dentry->d_name,
+                                      fhandle, fattr);
        if (error == -ENOENT) {
                if (nfs_server_capable(dir, NFS_CAP_CASE_INSENSITIVE))
                        dir_verifier = inode_peek_iversion_raw(dir);
@@ -2246,7 +2247,7 @@ nfs4_lookup_revalidate(struct inode *dir, const struct qstr *name,
 reval_dentry:
        if (flags & LOOKUP_RCU)
                return -ECHILD;
-       return nfs_lookup_revalidate_dentry(dir, dentry, inode, flags);
+       return nfs_lookup_revalidate_dentry(dir, name, dentry, inode, flags);
 
 full_reval:
        return nfs_do_lookup_revalidate(dir, name, dentry, flags);
@@ -2305,7 +2306,8 @@ nfs_add_or_obtain(struct dentry *dentry, struct nfs_fh *fhandle,
        d_drop(dentry);
 
        if (fhandle->size == 0) {
-               error = NFS_PROTO(dir)->lookup(dir, dentry, fhandle, fattr);
+               error = NFS_PROTO(dir)->lookup(dir, dentry, &dentry->d_name,
+                                              fhandle, fattr);
                if (error)
                        goto out_error;
        }

diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index 2d53574da6059093e89ee4761b930ec20dd10968..973aed9cc5fe97080e5e9b096e7758e982a95acf 100644 (file)
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -308,7 +308,7 @@ int nfs_submount(struct fs_context *fc, struct nfs_server *server)
        int err;
 
        /* Look it up again to get its attributes */
-       err = server->nfs_client->rpc_ops->lookup(d_inode(parent), dentry,
+       err = server->nfs_client->rpc_ops->lookup(d_inode(parent), dentry, &dentry->d_name,
                                                  ctx->mntfh, ctx->clone_data.fattr);
        dput(parent);
        if (err != 0)

diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index 1566163c6d85b998f0391ec48b3429d245a54651..ce70768e0201a0e2c058798533b436c2496f4a23 100644 (file)
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -192,7 +192,7 @@ __nfs3_proc_lookup(struct inode *dir, const char *name, size_t len,
 }
 
 static int
-nfs3_proc_lookup(struct inode *dir, struct dentry *dentry,
+nfs3_proc_lookup(struct inode *dir, struct dentry *dentry, const struct qstr *name,
                 struct nfs_fh *fhandle, struct nfs_fattr *fattr)
 {
        unsigned short task_flags = 0;
@@ -202,8 +202,7 @@ nfs3_proc_lookup(struct inode *dir, struct dentry *dentry,
                task_flags |= RPC_TASK_TIMEOUT;
 
        dprintk("NFS call  lookup %pd2\n", dentry);
-       return __nfs3_proc_lookup(dir, dentry->d_name.name,
-                                 dentry->d_name.len, fhandle, fattr,
+       return __nfs3_proc_lookup(dir, name->name, name->len, fhandle, fattr,
                                  task_flags);
 }
 

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 405f17e6e0b45b26cebae06c5bbe932895af9a56..4d85068e820d77a44168f8c573bfbb162e2851c4 100644 (file)
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4536,15 +4536,15 @@ nfs4_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,
 }
 
 static int _nfs4_proc_lookup(struct rpc_clnt *clnt, struct inode *dir,
-               struct dentry *dentry, struct nfs_fh *fhandle,
-               struct nfs_fattr *fattr)
+               struct dentry *dentry, const struct qstr *name,
+               struct nfs_fh *fhandle, struct nfs_fattr *fattr)
 {
        struct nfs_server *server = NFS_SERVER(dir);
        int                    status;
        struct nfs4_lookup_arg args = {
                .bitmask = server->attr_bitmask,
                .dir_fh = NFS_FH(dir),
-               .name = &dentry->d_name,
+               .name = name,
        };
        struct nfs4_lookup_res res = {
                .server = server,
@@ -4586,17 +4586,16 @@ static void nfs_fixup_secinfo_attributes(struct nfs_fattr *fattr)
 }
 
 static int nfs4_proc_lookup_common(struct rpc_clnt **clnt, struct inode *dir,
-                                  struct dentry *dentry, struct nfs_fh *fhandle,
-                                  struct nfs_fattr *fattr)
+                                  struct dentry *dentry, const struct qstr *name,
+                                  struct nfs_fh *fhandle, struct nfs_fattr *fattr)
 {
        struct nfs4_exception exception = {
                .interruptible = true,
        };
        struct rpc_clnt *client = *clnt;
-       const struct qstr *name = &dentry->d_name;
        int err;
        do {
-               err = _nfs4_proc_lookup(client, dir, dentry, fhandle, fattr);
+               err = _nfs4_proc_lookup(client, dir, dentry, name, fhandle, fattr);
                trace_nfs4_lookup(dir, name, err);
                switch (err) {
                case -NFS4ERR_BADNAME:
@@ -4631,13 +4630,13 @@ out:
        return err;
 }
 
-static int nfs4_proc_lookup(struct inode *dir, struct dentry *dentry,
+static int nfs4_proc_lookup(struct inode *dir, struct dentry *dentry, const struct qstr *name,
                            struct nfs_fh *fhandle, struct nfs_fattr *fattr)
 {
        int status;
        struct rpc_clnt *client = NFS_CLIENT(dir);
 
-       status = nfs4_proc_lookup_common(&client, dir, dentry, fhandle, fattr);
+       status = nfs4_proc_lookup_common(&client, dir, dentry, name, fhandle, fattr);
        if (client != NFS_CLIENT(dir)) {
                rpc_shutdown_client(client);
                nfs_fixup_secinfo_attributes(fattr);
@@ -4652,7 +4651,8 @@ nfs4_proc_lookup_mountpoint(struct inode *dir, struct dentry *dentry,
        struct rpc_clnt *client = NFS_CLIENT(dir);
        int status;
 
-       status = nfs4_proc_lookup_common(&client, dir, dentry, fhandle, fattr);
+       status = nfs4_proc_lookup_common(&client, dir, dentry, &dentry->d_name,
+                                        fhandle, fattr);
        if (status < 0)
                return ERR_PTR(status);
        return (client == NFS_CLIENT(dir)) ? rpc_clone_client(client) : client;

diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
index 6c09cd090c34c7df28c0907c4e0d962fac1aef6f..77920a2e3cef51073115c217d510064a5f94fb43 100644 (file)
--- a/fs/nfs/proc.c
+++ b/fs/nfs/proc.c
@@ -153,13 +153,13 @@ nfs_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,
 }
 
 static int
-nfs_proc_lookup(struct inode *dir, struct dentry *dentry,
+nfs_proc_lookup(struct inode *dir, struct dentry *dentry, const struct qstr *name,
                struct nfs_fh *fhandle, struct nfs_fattr *fattr)
 {
        struct nfs_diropargs    arg = {
                .fh             = NFS_FH(dir),
-               .name           = dentry->d_name.name,
-               .len            = dentry->d_name.len
+               .name           = name->name,
+               .len            = name->len
        };
        struct nfs_diropok      res = {
                .fh             = fhandle,

diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 559273a0f16d81ef93b66c088cc0c5f86ab3109c..08b62bbf59f0a397a00bce464d4cda191d58bf81 100644 (file)
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1785,7 +1785,7 @@ struct nfs_rpc_ops {
                            struct nfs_fattr *, struct inode *);
        int     (*setattr) (struct dentry *, struct nfs_fattr *,
                            struct iattr *);
-       int     (*lookup)  (struct inode *, struct dentry *,
+       int     (*lookup)  (struct inode *, struct dentry *, const struct qstr *,
                            struct nfs_fh *, struct nfs_fattr *);
        int     (*lookupp) (struct inode *, struct nfs_fh *,
                            struct nfs_fattr *);