nfsd: Set lc_size_chg before ops->proc_layoutcommit

If lookup_one_len() failed, nfsd should free those memory allocated for fname.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4recover.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

fs/nfsd/nfs4recover.c +9 -3

index d88ea7b..591bfbd 100644
@@ -272,6 +272,7 @@ nfsd4_list_rec_dir(recdir_func *f, struct nfsd_net *nn)
 		.ctx.actor = nfsd4_build_namelist,
 		.names = LIST_HEAD_INIT(ctx.names)
 	};
+	struct name_list *entry, *tmp;
 	int status;
 
 	status = nfs4_save_creds(&original_cred);

@@ -286,9 +287,8 @@ nfsd4_list_rec_dir(recdir_func *f, struct nfsd_net *nn)
 
 	status = iterate_dir(nn->rec_file, &ctx.ctx);
 	mutex_lock_nested(&d_inode(dir)->i_mutex, I_MUTEX_PARENT);
-	while (!list_empty(&ctx.names)) {
-		struct name_list *entry;
-		entry = list_entry(ctx.names.next, struct name_list, list);
+
+	list_for_each_entry_safe(entry, tmp, &ctx.names, list) {
 		if (!status) {
 			struct dentry *dentry;
 			dentry = lookup_one_len(entry->name, dir, HEXDIR_LEN-1);

@@ -304,6 +304,12 @@ nfsd4_list_rec_dir(recdir_func *f, struct nfsd_net *nn)
 	}
 	mutex_unlock(&d_inode(dir)->i_mutex);
 	nfs4_reset_creds(original_cred);
+
+	list_for_each_entry_safe(entry, tmp, &ctx.names, list) {
+		dprintk("NFSD: %s. Left entry %s\n", __func__, entry->name);
+		list_del(&entry->list);
+		kfree(entry);
+	}
 	return status;
 }
 

Signature · 1 line

2.4.3

When setting refer for exports entry,

[   88.414272] kernel BUG at fs/nfsd/nfs4xdr.c:2249!
[   88.414828] invalid opcode: 0000 [#1] SMP
[   88.415368] Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver nfs fscache nfsd xfs libcrc32c iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi iosf_mbi ppdev btrfs coretemp crct10dif_pclmul crc32_pclmul crc32c_intel xor ghash_clmulni_intel raid6_pq vmw_balloon parport_pc parport i2c_piix4 shpchp vmw_vmci acpi_cpufreq auth_rpcgss nfs_acl lockd grace sunrpc vmwgfx drm_kms_helper ttm drm mptspi mptscsih serio_raw mptbase e1000 scsi_transport_spi ata_generic pata_acpi [last unloaded: nfsd]
[   88.417827] CPU: 0 PID: 2116 Comm: nfsd Not tainted 4.0.7-300.fc22.x86_64 #1
[   88.418448] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 05/20/2014
[   88.419093] task: ffff880079146d50 ti: ffff8800785d8000 task.ti: ffff8800785d8000
[   88.419729] RIP: 0010:[<ffffffffa04b3c10>]  [<ffffffffa04b3c10>] nfsd4_encode_fattr+0x820/0x1f00 [nfsd]
[   88.420376] RSP: 0000:ffff8800785db998  EFLAGS: 00010206
[   88.421027] RAX: 0000000000000001 RBX: 000000000018091a RCX: ffff88006668b980
[   88.421676] RDX: 00000000fffef7fc RSI: 0000000000000000 RDI: ffff880078d05000
[   88.422315] RBP: ffff8800785dbb58 R08: ffff880078d043f8 R09: ffff880078d4a000
[   88.422968] R10: 0000000000010000 R11: 0000000000000002 R12: 0000000000b0a23a
[   88.423612] R13: ffff880078d05000 R14: ffff880078683100 R15: ffff88006668b980
[   88.424295] FS:  0000000000000000(0000) GS:ffff88007c600000(0000) knlGS:0000000000000000
[   88.424944] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   88.425597] CR2: 00007f40bc370f90 CR3: 0000000035af5000 CR4: 00000000001407f0
[   88.426285] Stack:
[   88.426921]  ffff8800785dbaa8 ffffffffa049e4af ffff8800785dba08 ffffffff813298f0
[   88.427585]  ffff880078683300 ffff8800769b0de8 0000089d00000001 0000000087f805e0
[   88.428228]  ffff880000000000 ffff880079434a00 0000000000000000 ffff88006668b980
[   88.428877] Call Trace:
[   88.429527]  [<ffffffffa049e4af>] ? exp_get_by_name+0x7f/0xb0 [nfsd]
[   88.430168]  [<ffffffff813298f0>] ? inode_doinit_with_dentry+0x210/0x6a0
[   88.430807]  [<ffffffff8123833e>] ? d_lookup+0x2e/0x60
[   88.431449]  [<ffffffff81236133>] ? dput+0x33/0x230
[   88.432097]  [<ffffffff8123f214>] ? mntput+0x24/0x40
[   88.432719]  [<ffffffff812272b2>] ? path_put+0x22/0x30
[   88.433340]  [<ffffffffa049ac87>] ? nfsd_cross_mnt+0xb7/0x1c0 [nfsd]
[   88.433954]  [<ffffffffa04b54e0>] nfsd4_encode_dirent+0x1b0/0x3d0 [nfsd]
[   88.434601]  [<ffffffffa04b5330>] ? nfsd4_encode_getattr+0x40/0x40 [nfsd]
[   88.435172]  [<ffffffffa049c991>] nfsd_readdir+0x1c1/0x2a0 [nfsd]
[   88.435710]  [<ffffffffa049a530>] ? nfsd_direct_splice_actor+0x20/0x20 [nfsd]
[   88.436447]  [<ffffffffa04abf30>] nfsd4_encode_readdir+0x120/0x220 [nfsd]
[   88.437011]  [<ffffffffa04b58cd>] nfsd4_encode_operation+0x7d/0x190 [nfsd]
[   88.437566]  [<ffffffffa04aa6dd>] nfsd4_proc_compound+0x24d/0x6f0 [nfsd]
[   88.438157]  [<ffffffffa0496103>] nfsd_dispatch+0xc3/0x220 [nfsd]
[   88.438680]  [<ffffffffa006f0cb>] svc_process_common+0x43b/0x690 [sunrpc]
[   88.439192]  [<ffffffffa0070493>] svc_process+0x103/0x1b0 [sunrpc]
[   88.439694]  [<ffffffffa0495a57>] nfsd+0x117/0x190 [nfsd]
[   88.440194]  [<ffffffffa0495940>] ? nfsd_destroy+0x90/0x90 [nfsd]
[   88.440697]  [<ffffffff810bb728>] kthread+0xd8/0xf0
[   88.441260]  [<ffffffff810bb650>] ? kthread_worker_fn+0x180/0x180
[   88.441762]  [<ffffffff81789e58>] ret_from_fork+0x58/0x90
[   88.442322]  [<ffffffff810bb650>] ? kthread_worker_fn+0x180/0x180
[   88.442879] Code: 0f 84 93 05 00 00 83 f8 ea c7 85 a0 fe ff ff 00 00 27 30 0f 84 ba fe ff ff 85 c0 0f 85 a5 fe ff ff e9 e3 f9 ff ff 0f 1f 44 00 00 <0f> 0b 66 0f 1f 44 00 00 be 04 00 00 00 4c 89 ef 4c 89 8d 68 fe
[   88.444052] RIP  [<ffffffffa04b3c10>] nfsd4_encode_fattr+0x820/0x1f00 [nfsd]
[   88.444658]  RSP <ffff8800785db998>
[   88.445232] ---[ end trace 6cb9d0487d94a29f ]---

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4xdr.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

fs/nfsd/nfs4xdr.c +6 -5

index 5463385..75e0563 100644
@@ -2143,6 +2143,7 @@ nfsd4_encode_aclname(struct xdr_stream *xdr, struct svc_rqst *rqstp,
 #define WORD0_ABSENT_FS_ATTRS (FATTR4_WORD0_FS_LOCATIONS | FATTR4_WORD0_FSID | \
 			      FATTR4_WORD0_RDATTR_ERROR)
 #define WORD1_ABSENT_FS_ATTRS FATTR4_WORD1_MOUNTED_ON_FILEID
+#define WORD2_ABSENT_FS_ATTRS 0
 
 #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
 static inline __be32

@@ -2171,7 +2172,7 @@ nfsd4_encode_security_label(struct xdr_stream *xdr, struct svc_rqst *rqstp,
 { return 0; }
 #endif
 
-static __be32 fattr_handle_absent_fs(u32 *bmval0, u32 *bmval1, u32 *rdattr_err)
+static __be32 fattr_handle_absent_fs(u32 *bmval0, u32 *bmval1, u32 *bmval2, u32 *rdattr_err)
 {
 	/* As per referral draft:  */
 	if (*bmval0 & ~WORD0_ABSENT_FS_ATTRS ||

@@ -2184,6 +2185,7 @@ static __be32 fattr_handle_absent_fs(u32 *bmval0, u32 *bmval1, u32 *rdattr_err)
 	}
 	*bmval0 &= WORD0_ABSENT_FS_ATTRS;
 	*bmval1 &= WORD1_ABSENT_FS_ATTRS;
+	*bmval2 &= WORD2_ABSENT_FS_ATTRS;
 	return 0;
 }
 

@@ -2246,8 +2248,7 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 	BUG_ON(bmval2 & ~nfsd_suppattrs2(minorversion));
 
 	if (exp->ex_fslocs.migrated) {
-		BUG_ON(bmval[2]);
-		status = fattr_handle_absent_fs(&bmval0, &bmval1, &rdattr_err);
+		status = fattr_handle_absent_fs(&bmval0, &bmval1, &bmval2, &rdattr_err);
 		if (status)
 			goto out;
 	}

@@ -2286,8 +2287,8 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 	}
 
 #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
-	if ((bmval[2] & FATTR4_WORD2_SECURITY_LABEL) ||
-			bmval[0] & FATTR4_WORD0_SUPPORTED_ATTRS) {
+	if ((bmval2 & FATTR4_WORD2_SECURITY_LABEL) ||
+	     bmval0 & FATTR4_WORD0_SUPPORTED_ATTRS) {
 		err = security_inode_getsecctx(d_inode(dentry),
 						&context, &contextlen);
 		contextsupport = (err == 0);

Signature · 1 line

2.4.3

After proc_layoutcommit success, i_size_read(inode) always >= new_size. Just set lc_size_chg before proc_layoutcommit, if proc_layoutcommit failed, nfsd will skip the lc_size_chg, so it's no harm.

Signed-off-by Kinglong Mee <[email protected]>

---
 fs/nfsd/nfs4proc.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

fs/nfsd/nfs4proc.c +1 -5

index 90cfda7..d112c8a 100644
@@ -1364,10 +1364,6 @@ nfsd4_layoutcommit(struct svc_rqst *rqstp,
 		goto out;
 	}
 
-	nfserr = ops->proc_layoutcommit(inode, lcp);
-	if (nfserr)
-		goto out_put_stid;
-
 	if (new_size > i_size_read(inode)) {
 		lcp->lc_size_chg = 1;
 		lcp->lc_newsize = new_size;

@@ -1375,7 +1371,7 @@ nfsd4_layoutcommit(struct svc_rqst *rqstp,
 		lcp->lc_size_chg = 0;
 	}
 
-out_put_stid:
+	nfserr = ops->proc_layoutcommit(inode, lcp);
 	nfs4_put_stid(&ls->ls_stid);
 out:
 	return nfserr;

Signature · 1 line

2.4.3

Quoted 3 lines · Kinglong Mee

After proc_layoutcommit success, i_size_read(inode) always >= new_size. Just set lc_size_chg before proc_layoutcommit, if proc_layoutcommit failed, nfsd will skip the lc_size_chg, so it's no harm.

Looks right to me, though could probably use an ACK from Christoph.

(But looking at the spec I'm confused about how the client is supposed to use this information.)

Quoted 5 lines · J. Bruce Fields

Quoted 3 lines · Kinglong Mee

After proc_layoutcommit success, i_size_read(inode) always >= new_size. Just set lc_size_chg before proc_layoutcommit, if proc_layoutcommit failed, nfsd will skip the lc_size_chg, so it's no harm.

Quoted 4 lines

Looks right to me, though could probably use an ACK from Christoph.

(But looking at the spec I'm confused about how the client is supposed to use this information.)

I don;t think its possible to use, as there it is inherently racy.

If the test passes xfstests I'm fine with it.

Quoted 5 lines · J. Bruce Fields

Quoted 3 lines · Kinglong Mee

After proc_layoutcommit success, i_size_read(inode) always >= new_size. Just set lc_size_chg before proc_layoutcommit, if proc_layoutcommit failed, nfsd will skip the lc_size_chg, so it's no harm.

Quoted 4 lines

Looks right to me, though could probably use an ACK from Christoph.

(But looking at the spec I'm confused about how the client is supposed to use this information.)

I don;t think its possible to use, as there it is inherently racy.

If the test passes xfstests I'm fine with it.