Skip to contentSkip to conversation

NFSD: Encode only the status in NFS-ACL v2 GETACL error replies

PATCH New Chuck Lever 1 participant 1 message Jul 12

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

Chuck Lever

author maintainer
#0 lore raw

PATCH 1/1 NFSD: Encode only the status in NFS-ACL v2 GETACL error replies

The NFSv2 ACL GETACL reply is a union that carries file attributes and ACL data only when the status is NFS_OK. All error cases are void results. However, currently the NFSv2 ACL GETACL result encoder decides whether to append the "OK" body by testing only whether the file handle resolved to a positive dentry, not the actual reply status.

A GETACL request that resolves its file handle but then fails for another reason (an unsupported mask value, a getattr failure, or an ACL retrieval error) therefore appends file attributes and ACL data after the error status on the wire. Worse, when the mask is rejected, fh_getattr() hasn't been called at all, so those attributes are serialized from a zero-filled kstat and are junk.

The logic before the xdr_stream conversion used the reply status. Revert to that approach (but keep the xdr_stream conversion in place).

Fixes f8cba47344f7 ("NFSD: Update the NFSv2 GETACL result encoder to use struct xdr_stream")
Signed-off-by Chuck Lever <[email protected]>
---
 fs/nfsd/nfs2acl.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)
fs/nfsd/nfs2acl.c +15 -16
index 827f90194c43..2998640f259d 100644
@@ -253,22 +253,21 @@ nfsaclsvc_encode_getaclres(struct svc_rqst *rqstp, struct xdr_stream *xdr)
 
 	if (!svcxdr_encode_stat(xdr, resp->status))
 		return false;
-
-	if (dentry == NULL || d_really_is_negative(dentry))
-		return true;
-	inode = d_inode(dentry);
-
-	if (!svcxdr_encode_fattr(rqstp, xdr, &resp->fh, &resp->stat))
-		return false;
-	if (xdr_stream_encode_u32(xdr, resp->mask) < 0)
-		return false;
-
-	if (!nfs_stream_encode_acl(xdr, inode, resp->acl_access,
-				   resp->mask & NFS_ACL, 0))
-		return false;
-	if (!nfs_stream_encode_acl(xdr, inode, resp->acl_default,
-				   resp->mask & NFS_DFACL, NFS_ACL_DEFAULT))
-		return false;
+	switch (resp->status) {
+	case nfs_ok:
+		inode = d_inode(dentry);
+		if (!svcxdr_encode_fattr(rqstp, xdr, &resp->fh, &resp->stat))
+			return false;
+		if (xdr_stream_encode_u32(xdr, resp->mask) < 0)
+			return false;
+		if (!nfs_stream_encode_acl(xdr, inode, resp->acl_access,
+					   resp->mask & NFS_ACL, 0))
+			return false;
+		if (!nfs_stream_encode_acl(xdr, inode, resp->acl_default,
+					   resp->mask & NFS_DFACL, NFS_ACL_DEFAULT))
+			return false;
+		break;
+	}
 
 	return true;
 }
Signature · 1 line
2.54.0