Skip to content

Commit

Permalink
nfs: fix ->d_revalidate() UAF on ->d_name accesses
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Al Viro committed Jan 28, 2025
1 parent 39f644a commit ffeeaad
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 24 deletions.
14 changes: 8 additions & 6 deletions fs/nfs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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;

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion fs/nfs/namespace.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions fs/nfs/nfs3proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down
20 changes: 10 additions & 10 deletions fs/nfs/nfs4proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -4631,13 +4630,13 @@ static int nfs4_proc_lookup_common(struct rpc_clnt **clnt, struct inode *dir,
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);
Expand All @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions fs/nfs/proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion include/linux/nfs_xdr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);
Expand Down

0 comments on commit ffeeaad

Please sign in to comment.