Skip to content

Commit

Permalink
NFSv4: Fix free of uninitialized nfs4_label on referral lookup.
Browse files Browse the repository at this point in the history
commit c3ed222 upstream.

Send along the already-allocated fattr along with nfs4_fs_locations, and
drop the memcpy of fattr.  We end up growing two more allocations, but this
fixes up a crash as:

PID: 790    TASK: ffff88811b43c000  CPU: 0   COMMAND: "ls"
 #0 [ffffc90000857920] panic at ffffffff81b9bfde
 #1 [ffffc900008579c0] do_trap at ffffffff81023a9b
 #2 [ffffc90000857a10] do_error_trap at ffffffff81023b78
 #3 [ffffc90000857a58] exc_stack_segment at ffffffff81be1f45
 #4 [ffffc90000857a80] asm_exc_stack_segment at ffffffff81c009de
 #5 [ffffc90000857b08] nfs_lookup at ffffffffa0302322 [nfs]
 #6 [ffffc90000857b70] __lookup_slow at ffffffff813a4a5f
 #7 [ffffc90000857c60] walk_component at ffffffff813a86c4
 #8 [ffffc90000857cb8] path_lookupat at ffffffff813a9553
 #9 [ffffc90000857cf0] filename_lookup at ffffffff813ab86b

Suggested-by: Trond Myklebust <trondmy@hammerspace.com>
Fixes: 9558a00 ("NFS: Remove the label from the nfs4_lookup_res struct")
Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Benjamin Coddington authored and gregkh committed Nov 3, 2022
1 parent 4f5365f commit eb1fe96
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 14 deletions.
9 changes: 7 additions & 2 deletions fs/nfs/nfs4namespace.c
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,9 @@ static int nfs_do_refmount(struct fs_context *fc, struct rpc_clnt *client)
fs_locations = kmalloc(sizeof(struct nfs4_fs_locations), GFP_KERNEL);
if (!fs_locations)
goto out_free;
fs_locations->fattr = nfs_alloc_fattr();
if (!fs_locations->fattr)
goto out_free_2;

/* Get locations */
dentry = ctx->clone_data.dentry;
Expand All @@ -427,14 +430,16 @@ static int nfs_do_refmount(struct fs_context *fc, struct rpc_clnt *client)
err = nfs4_proc_fs_locations(client, d_inode(parent), &dentry->d_name, fs_locations, page);
dput(parent);
if (err != 0)
goto out_free_2;
goto out_free_3;

err = -ENOENT;
if (fs_locations->nlocations <= 0 ||
fs_locations->fs_path.ncomponents <= 0)
goto out_free_2;
goto out_free_3;

err = nfs_follow_referral(fc, fs_locations);
out_free_3:
kfree(fs_locations->fattr);
out_free_2:
kfree(fs_locations);
out_free:
Expand Down
15 changes: 7 additions & 8 deletions fs/nfs/nfs4proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -4213,6 +4213,8 @@ static int nfs4_get_referral(struct rpc_clnt *client, struct inode *dir,
if (locations == NULL)
goto out;

locations->fattr = fattr;

status = nfs4_proc_fs_locations(client, dir, name, locations, page);
if (status != 0)
goto out;
Expand All @@ -4222,17 +4224,14 @@ static int nfs4_get_referral(struct rpc_clnt *client, struct inode *dir,
* referral. Cause us to drop into the exception handler, which
* will kick off migration recovery.
*/
if (nfs_fsid_equal(&NFS_SERVER(dir)->fsid, &locations->fattr.fsid)) {
if (nfs_fsid_equal(&NFS_SERVER(dir)->fsid, &fattr->fsid)) {
dprintk("%s: server did not return a different fsid for"
" a referral at %s\n", __func__, name->name);
status = -NFS4ERR_MOVED;
goto out;
}
/* Fixup attributes for the nfs_lookup() call to nfs_fhget() */
nfs_fixup_referral_attributes(&locations->fattr);

/* replace the lookup nfs_fattr with the locations nfs_fattr */
memcpy(fattr, &locations->fattr, sizeof(struct nfs_fattr));
nfs_fixup_referral_attributes(fattr);
memset(fhandle, 0, sizeof(struct nfs_fh));
out:
if (page)
Expand Down Expand Up @@ -7917,7 +7916,7 @@ static int _nfs4_proc_fs_locations(struct rpc_clnt *client, struct inode *dir,
else
bitmask[1] &= ~FATTR4_WORD1_MOUNTED_ON_FILEID;

nfs_fattr_init(&fs_locations->fattr);
nfs_fattr_init(fs_locations->fattr);
fs_locations->server = server;
fs_locations->nlocations = 0;
status = nfs4_call_sync(client, server, &msg, &args.seq_args, &res.seq_res, 0);
Expand Down Expand Up @@ -7982,7 +7981,7 @@ static int _nfs40_proc_get_locations(struct nfs_server *server,
unsigned long now = jiffies;
int status;

nfs_fattr_init(&locations->fattr);
nfs_fattr_init(locations->fattr);
locations->server = server;
locations->nlocations = 0;

Expand Down Expand Up @@ -8035,7 +8034,7 @@ static int _nfs41_proc_get_locations(struct nfs_server *server,
};
int status;

nfs_fattr_init(&locations->fattr);
nfs_fattr_init(locations->fattr);
locations->server = server;
locations->nlocations = 0;

Expand Down
9 changes: 8 additions & 1 deletion fs/nfs/nfs4state.c
Original file line number Diff line number Diff line change
Expand Up @@ -2096,6 +2096,11 @@ static int nfs4_try_migration(struct nfs_server *server, const struct cred *cred
dprintk("<-- %s: no memory\n", __func__);
goto out;
}
locations->fattr = nfs_alloc_fattr();
if (locations->fattr == NULL) {
dprintk("<-- %s: no memory\n", __func__);
goto out;
}

inode = d_inode(server->super->s_root);
result = nfs4_proc_get_locations(server, NFS_FH(inode), locations,
Expand All @@ -2110,7 +2115,7 @@ static int nfs4_try_migration(struct nfs_server *server, const struct cred *cred
if (!locations->nlocations)
goto out;

if (!(locations->fattr.valid & NFS_ATTR_FATTR_V4_LOCATIONS)) {
if (!(locations->fattr->valid & NFS_ATTR_FATTR_V4_LOCATIONS)) {
dprintk("<-- %s: No fs_locations data, migration skipped\n",
__func__);
goto out;
Expand All @@ -2135,6 +2140,8 @@ static int nfs4_try_migration(struct nfs_server *server, const struct cred *cred
out:
if (page != NULL)
__free_page(page);
if (locations != NULL)
kfree(locations->fattr);
kfree(locations);
if (result) {
pr_err("NFS: migration recovery failed (server %s)\n",
Expand Down
4 changes: 2 additions & 2 deletions fs/nfs/nfs4xdr.c
Original file line number Diff line number Diff line change
Expand Up @@ -7028,7 +7028,7 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req,
if (res->migration) {
xdr_enter_page(xdr, PAGE_SIZE);
status = decode_getfattr_generic(xdr,
&res->fs_locations->fattr,
res->fs_locations->fattr,
NULL, res->fs_locations,
NULL, res->fs_locations->server);
if (status)
Expand All @@ -7041,7 +7041,7 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req,
goto out;
xdr_enter_page(xdr, PAGE_SIZE);
status = decode_getfattr_generic(xdr,
&res->fs_locations->fattr,
res->fs_locations->fattr,
NULL, res->fs_locations,
NULL, res->fs_locations->server);
}
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 @@ -1219,7 +1219,7 @@ struct nfs4_fs_location {

#define NFS4_FS_LOCATIONS_MAXENTRIES 10
struct nfs4_fs_locations {
struct nfs_fattr fattr;
struct nfs_fattr *fattr;
const struct nfs_server *server;
struct nfs4_pathname fs_path;
int nlocations;
Expand Down

0 comments on commit eb1fe96

Please sign in to comment.