Skip to content

Commit c3ed222

Browse files
bcodding-rhamschuma-ntap
authored andcommitted
NFSv4: Fix free of uninitialized nfs4_label on referral lookup.
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>
1 parent 9c4a5c7 commit c3ed222

File tree

5 files changed

+25
-14
lines changed

5 files changed

+25
-14
lines changed

fs/nfs/nfs4namespace.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,9 @@ static int nfs_do_refmount(struct fs_context *fc, struct rpc_clnt *client)
417417
fs_locations = kmalloc(sizeof(struct nfs4_fs_locations), GFP_KERNEL);
418418
if (!fs_locations)
419419
goto out_free;
420+
fs_locations->fattr = nfs_alloc_fattr();
421+
if (!fs_locations->fattr)
422+
goto out_free_2;
420423

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

432435
err = -ENOENT;
433436
if (fs_locations->nlocations <= 0 ||
434437
fs_locations->fs_path.ncomponents <= 0)
435-
goto out_free_2;
438+
goto out_free_3;
436439

437440
err = nfs_follow_referral(fc, fs_locations);
441+
out_free_3:
442+
kfree(fs_locations->fattr);
438443
out_free_2:
439444
kfree(fs_locations);
440445
out_free:

fs/nfs/nfs4proc.c

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4247,6 +4247,8 @@ static int nfs4_get_referral(struct rpc_clnt *client, struct inode *dir,
42474247
if (locations == NULL)
42484248
goto out;
42494249

4250+
locations->fattr = fattr;
4251+
42504252
status = nfs4_proc_fs_locations(client, dir, name, locations, page);
42514253
if (status != 0)
42524254
goto out;
@@ -4256,17 +4258,14 @@ static int nfs4_get_referral(struct rpc_clnt *client, struct inode *dir,
42564258
* referral. Cause us to drop into the exception handler, which
42574259
* will kick off migration recovery.
42584260
*/
4259-
if (nfs_fsid_equal(&NFS_SERVER(dir)->fsid, &locations->fattr.fsid)) {
4261+
if (nfs_fsid_equal(&NFS_SERVER(dir)->fsid, &fattr->fsid)) {
42604262
dprintk("%s: server did not return a different fsid for"
42614263
" a referral at %s\n", __func__, name->name);
42624264
status = -NFS4ERR_MOVED;
42634265
goto out;
42644266
}
42654267
/* Fixup attributes for the nfs_lookup() call to nfs_fhget() */
4266-
nfs_fixup_referral_attributes(&locations->fattr);
4267-
4268-
/* replace the lookup nfs_fattr with the locations nfs_fattr */
4269-
memcpy(fattr, &locations->fattr, sizeof(struct nfs_fattr));
4268+
nfs_fixup_referral_attributes(fattr);
42704269
memset(fhandle, 0, sizeof(struct nfs_fh));
42714270
out:
42724271
if (page)
@@ -7979,7 +7978,7 @@ static int _nfs4_proc_fs_locations(struct rpc_clnt *client, struct inode *dir,
79797978
else
79807979
bitmask[1] &= ~FATTR4_WORD1_MOUNTED_ON_FILEID;
79817980

7982-
nfs_fattr_init(&fs_locations->fattr);
7981+
nfs_fattr_init(fs_locations->fattr);
79837982
fs_locations->server = server;
79847983
fs_locations->nlocations = 0;
79857984
status = nfs4_call_sync(client, server, &msg, &args.seq_args, &res.seq_res, 0);
@@ -8044,7 +8043,7 @@ static int _nfs40_proc_get_locations(struct nfs_server *server,
80448043
unsigned long now = jiffies;
80458044
int status;
80468045

8047-
nfs_fattr_init(&locations->fattr);
8046+
nfs_fattr_init(locations->fattr);
80488047
locations->server = server;
80498048
locations->nlocations = 0;
80508049

@@ -8109,7 +8108,7 @@ static int _nfs41_proc_get_locations(struct nfs_server *server,
81098108
};
81108109
int status;
81118110

8112-
nfs_fattr_init(&locations->fattr);
8111+
nfs_fattr_init(locations->fattr);
81138112
locations->server = server;
81148113
locations->nlocations = 0;
81158114

fs/nfs/nfs4state.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2116,6 +2116,11 @@ static int nfs4_try_migration(struct nfs_server *server, const struct cred *cred
21162116
dprintk("<-- %s: no memory\n", __func__);
21172117
goto out;
21182118
}
2119+
locations->fattr = nfs_alloc_fattr();
2120+
if (locations->fattr == NULL) {
2121+
dprintk("<-- %s: no memory\n", __func__);
2122+
goto out;
2123+
}
21192124

21202125
inode = d_inode(server->super->s_root);
21212126
result = nfs4_proc_get_locations(server, NFS_FH(inode), locations,
@@ -2130,7 +2135,7 @@ static int nfs4_try_migration(struct nfs_server *server, const struct cred *cred
21302135
if (!locations->nlocations)
21312136
goto out;
21322137

2133-
if (!(locations->fattr.valid & NFS_ATTR_FATTR_V4_LOCATIONS)) {
2138+
if (!(locations->fattr->valid & NFS_ATTR_FATTR_V4_LOCATIONS)) {
21342139
dprintk("<-- %s: No fs_locations data, migration skipped\n",
21352140
__func__);
21362141
goto out;
@@ -2155,6 +2160,8 @@ static int nfs4_try_migration(struct nfs_server *server, const struct cred *cred
21552160
out:
21562161
if (page != NULL)
21572162
__free_page(page);
2163+
if (locations != NULL)
2164+
kfree(locations->fattr);
21582165
kfree(locations);
21592166
if (result) {
21602167
pr_err("NFS: migration recovery failed (server %s)\n",

fs/nfs/nfs4xdr.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7080,7 +7080,7 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req,
70807080
if (res->migration) {
70817081
xdr_enter_page(xdr, PAGE_SIZE);
70827082
status = decode_getfattr_generic(xdr,
7083-
&res->fs_locations->fattr,
7083+
res->fs_locations->fattr,
70847084
NULL, res->fs_locations,
70857085
res->fs_locations->server);
70867086
if (status)
@@ -7093,7 +7093,7 @@ static int nfs4_xdr_dec_fs_locations(struct rpc_rqst *req,
70937093
goto out;
70947094
xdr_enter_page(xdr, PAGE_SIZE);
70957095
status = decode_getfattr_generic(xdr,
7096-
&res->fs_locations->fattr,
7096+
res->fs_locations->fattr,
70977097
NULL, res->fs_locations,
70987098
res->fs_locations->server);
70997099
}

include/linux/nfs_xdr.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1222,7 +1222,7 @@ struct nfs4_fs_location {
12221222

12231223
#define NFS4_FS_LOCATIONS_MAXENTRIES 10
12241224
struct nfs4_fs_locations {
1225-
struct nfs_fattr fattr;
1225+
struct nfs_fattr *fattr;
12261226
const struct nfs_server *server;
12271227
struct nfs4_pathname fs_path;
12281228
int nlocations;

0 commit comments

Comments
 (0)