Linux NFS development
 help / color / mirror / Atom feed
From: Chuck Lever <cel@kernel.org>
To: NeilBrown <neil@brown.name>, Jeff Layton <jlayton@kernel.org>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <dai.ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Cc: <linux-nfs@vger.kernel.org>
Subject: [PATCH v1 14/17] NFSD: Use xdrgen XDR functions for the NFS-ACL v2 SETACL procedure
Date: Thu, 22 Oct 2026 09:53:38 -0400	[thread overview]
Message-ID: <20261022135341.14265-15-cel@kernel.org> (raw)
In-Reply-To: <20261022135341.14265-1-cel@kernel.org>

Convert the NFS-ACL version 2 SETACL procedure to xdrgen. Because
the reply carries only a status word and, on success, the file
attributes, it is emitted entirely by the generated
nfs_acl_svc_encode_SETACL2res(); unlike the GETACL conversion, no
hand-written reply tail is needed. A result wrapper bridges the
generated SETACL2res type and the scratch kstat that the NFSD VFS
layer fills.

The argument decoder is generated in full. SETACL2args carries a
secattr whose aclent arrays must become a struct posix_acl for the
VFS layer; the hook-driven aggregate codec added earlier performs
that per-entry transformation, so nfs_acl_svc_decode_SETACL2args()
owns the wire framing and drives the application hooks. An argument
wrapper pairs the generated SETACL2args with the decoded posix_acl
pointers and an "invalid" flag.

pc_argzero covers the whole argument wrapper, so the buffer is
zeroed before decode: the two posix_acl pointers start NULL, as the
unconditional release callback requires, and the "invalid" flag
starts clear.

The reply no longer carries a file handle, so the procedure
verifies the handle, sets the ACLs, captures the resulting
attributes, and releases the handle itself; the release callback
shrinks to the two posix_acl_release() calls for the decoded
argument ACLs. When the decoder has marked the argument ACL invalid
(see the codec patch), the procedure reports ACL2ERR_INVAL before
authorization and does nothing else. A default ACL presented for an
object that is not a directory is reported as ACL2ERR_INVAL once
the handle is verified, where set_posix_acl() would have reported
EACCES; draft-ietf-nfsv4-nfs-acl Section 3.4.4.2 lists that case
among the invalid ACLs. Filling the fattr reuses
nfsacld_stat_to_fattr() from the GETACL conversion, so the wire
result matches what the NFSv2 GETATTR encoder already produces.

Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/nfsd/nfs2acl.c | 120 +++++++++++++++++++++++-----------------------
 1 file changed, 60 insertions(+), 60 deletions(-)

diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c
index 8f0be7208e89..24ec141796b4 100644
--- a/fs/nfsd/nfs2acl.c
+++ b/fs/nfsd/nfs2acl.c
@@ -80,13 +80,13 @@ struct SETACL2args_wrapper {
 
 static_assert(offsetof(struct SETACL2args_wrapper, xdrgen) == 0);
 
-struct nfsd3_setaclargs {
-	struct svc_fh		fh;
-	__u32			mask;
-	struct posix_acl	*acl_access;
-	struct posix_acl	*acl_default;
+struct SETACL2res_wrapper {
+	struct SETACL2res	xdrgen;
+	struct kstat		stat;
 };
 
+static_assert(offsetof(struct SETACL2res_wrapper, xdrgen) == 0);
+
 struct nfsd3_accessargs {
 	struct svc_fh		fh;
 	__u32			access;
@@ -397,41 +397,59 @@ static __be32 nfsacld_proc_getacl(struct svc_rqst *rqstp)
 	goto out;
 }
 
-/*
- * Set the Access and/or Default ACL of a file.
+/**
+ * nfsacld_proc_setacl - SETACL: Set the Access and/or Default ACL of a file
+ * @rqstp: RPC transaction context
+ *
+ * Return:
+ *   %rpc_success:		RPC executed successfully
+ *
+ * RPC synopsis:
+ *   SETACL2res ACLPROC2_SETACL(SETACL2args) = 2;
  */
 static __be32 nfsacld_proc_setacl(struct svc_rqst *rqstp)
 {
-	struct nfsd3_setaclargs *argp = rqstp->rq_argp;
-	struct nfsd_attrstat *resp = rqstp->rq_resp;
+	struct SETACL2args_wrapper *argp = rqstp->rq_argp;
+	struct SETACL2res_wrapper *resp = rqstp->rq_resp;
+	struct svc_fh *fhp = &argp->fh;
 	struct inode *inode;
-	svc_fh *fh;
 	int error;
 
-	dprintk("nfsd: SETACL(2acl)   %s\n", SVCFH_fmt(&argp->fh));
+	nfsacld_fhandle_to_svc_fh(fhp, &argp->xdrgen.fh);
+	dprintk("nfsd: SETACL(2acl)   %s\n", SVCFH_fmt(fhp));
 
-	fh = fh_copy(&resp->fh, &argp->fh);
-	resp->status = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_SATTR);
-	if (resp->status != acl2_ok)
+	/* A well-formed but invalid ACL is rejected before authorization. */
+	if (argp->invalid) {
+		resp->xdrgen.status = acl2err_inval;
 		goto out;
+	}
 
-	inode = d_inode(fh->fh_dentry);
+	resp->xdrgen.status = fh_verify(rqstp, fhp, 0, NFSD_MAY_SATTR);
+	if (resp->xdrgen.status != acl2_ok)
+		goto out;
+	inode = d_inode(fhp->fh_dentry);
 
-	error = fh_want_write(fh);
+	/* The VFS reports a default ACL on a non-directory as EACCES. */
+	if (argp->acl_default && !S_ISDIR(inode->i_mode)) {
+		resp->xdrgen.status = acl2err_inval;
+		goto out;
+	}
+
+	error = fh_want_write(fhp);
 	if (error)
 		goto out_errno;
 
 	inode_lock(inode);
 
 	error = 0;
-	if (argp->mask & NA_ACL) {
-		error = set_posix_acl(&nop_mnt_idmap, fh->fh_dentry,
+	if (argp->xdrgen.acl.mask & NA_ACL) {
+		error = set_posix_acl(&nop_mnt_idmap, fhp->fh_dentry,
 				      ACL_TYPE_ACCESS, argp->acl_access);
 		if (error)
 			goto out_drop_lock;
 	}
-	if (argp->mask & NA_DFACL) {
-		error = set_posix_acl(&nop_mnt_idmap, fh->fh_dentry,
+	if (argp->xdrgen.acl.mask & NA_DFACL) {
+		error = set_posix_acl(&nop_mnt_idmap, fhp->fh_dentry,
 				      ACL_TYPE_DEFAULT, argp->acl_default);
 		if (error)
 			goto out_drop_lock;
@@ -439,20 +457,24 @@ static __be32 nfsacld_proc_setacl(struct svc_rqst *rqstp)
 
 	inode_unlock(inode);
 
-	fh_drop_write(fh);
+	fh_drop_write(fhp);
 
-	resp->status = fh_getattr(fh, &resp->stat);
+	resp->xdrgen.status = fh_getattr(fhp, &resp->stat);
 
 out:
+	if (resp->xdrgen.status == acl2_ok)
+		nfsacld_stat_to_fattr(rqstp, &resp->xdrgen.u.resok.attr,
+				      &resp->stat, fhp);
 	/* argp->acl_{access,default} are released in nfsaclsvc_release_setacl. */
-	resp->status = nfsacld_map_status(resp->status);
+	fh_put(fhp);
+	resp->xdrgen.status = nfsacld_map_status(resp->xdrgen.status);
 	return rpc_success;
 
 out_drop_lock:
 	inode_unlock(inode);
-	fh_drop_write(fh);
+	fh_drop_write(fhp);
 out_errno:
-	resp->status = nfsacld_map_errno(error);
+	resp->xdrgen.status = nfsacld_map_errno(error);
 	goto out;
 }
 
@@ -699,27 +721,6 @@ bool nfs_acl2_secattr_decode_end(struct xdrgen_aggregate_cursor *c, bool ok)
  * XDR decode functions
  */
 
-static bool
-nfsaclsvc_decode_setaclargs(struct svc_rqst *rqstp, struct xdr_stream *xdr)
-{
-	struct nfsd3_setaclargs *argp = rqstp->rq_argp;
-
-	if (!svcxdr_decode_fhandle(xdr, &argp->fh))
-		return false;
-	if (xdr_stream_decode_u32(xdr, &argp->mask) < 0)
-		return false;
-	if (argp->mask & ~NFS_ACL_MASK)
-		return false;
-	if (!nfs_stream_decode_acl(xdr, NULL, (argp->mask & NA_ACL) ?
-				   &argp->acl_access : NULL))
-		return false;
-	if (!nfs_stream_decode_acl(xdr, NULL, (argp->mask & NA_DFACL) ?
-				   &argp->acl_default : NULL))
-		return false;
-
-	return true;
-}
-
 static bool
 nfsaclsvc_decode_accessargs(struct svc_rqst *rqstp, struct xdr_stream *xdr)
 {
@@ -777,10 +778,8 @@ static void nfsaclsvc_release_access(struct svc_rqst *rqstp)
 
 static void nfsaclsvc_release_setacl(struct svc_rqst *rqstp)
 {
-	struct nfsd3_setaclargs *argp = rqstp->rq_argp;
-	struct nfsd_attrstat *resp = rqstp->rq_resp;
+	struct SETACL2args_wrapper *argp = rqstp->rq_argp;
 
-	fh_put(&resp->fh);
 	posix_acl_release(argp->acl_access);
 	posix_acl_release(argp->acl_default);
 }
@@ -813,16 +812,16 @@ static const struct svc_procedure nfsd_acl_procedures2[5] = {
 		.pc_name	= "GETACL",
 	},
 	[ACLPROC2_SETACL] = {
-		.pc_func = nfsacld_proc_setacl,
-		.pc_decode = nfsaclsvc_decode_setaclargs,
-		.pc_encode = nfssvc_encode_attrstatres,
-		.pc_release = nfsaclsvc_release_setacl,
-		.pc_argsize = sizeof(struct nfsd3_setaclargs),
-		.pc_argzero = sizeof(struct nfsd3_setaclargs),
-		.pc_ressize = sizeof(struct nfsd_attrstat),
-		.pc_cachetype = RC_NOCACHE,
-		.pc_xdrressize = ST+AT,
-		.pc_name = "SETACL",
+		.pc_func	= nfsacld_proc_setacl,
+		.pc_decode	= nfs_acl_svc_decode_SETACL2args,
+		.pc_encode	= nfs_acl_svc_encode_SETACL2res,
+		.pc_release	= nfsaclsvc_release_setacl,
+		.pc_argsize	= sizeof(struct SETACL2args_wrapper),
+		.pc_argzero	= sizeof(struct SETACL2args_wrapper),
+		.pc_ressize	= sizeof(struct SETACL2res_wrapper),
+		.pc_cachetype	= RC_NOCACHE,
+		.pc_xdrressize	= NFS_ACL2_SETACL2res_sz,
+		.pc_name	= "SETACL",
 	},
 	[ACLPROC2_GETATTR] = {
 		.pc_func = nfsacld_proc_getattr,
@@ -855,10 +854,11 @@ static const struct svc_procedure nfsd_acl_procedures2[5] = {
  */
 union nfsd_acl2_xdrstore {
 	struct GETACL2args_wrapper	getaclargs;
-	struct nfsd3_setaclargs		setaclargs;
+	struct SETACL2args_wrapper	setaclargs;
 	struct nfsd3_accessargs		accessargs;
 	struct nfsd_fhandle		fhandle;
 	struct GETACL2res_wrapper	getaclres;
+	struct SETACL2res_wrapper	setaclres;
 	struct nfsd_attrstat		attrstat;
 	struct nfsd3_accessres		accessres;
 };
-- 
2.56.0


  parent reply	other threads:[~2026-10-22 13:53 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-22 13:53 [PATCH v1 00/17] Convert server-side NFS_ACLv2 XDR to use xdrgen Chuck Lever
2026-10-22 13:53 ` [PATCH v1 01/17] xdrgen: Reject out-of-range unsigned short values on decode Chuck Lever
2026-10-22 13:53 ` [PATCH v1 02/17] xdrgen: Reject out-of-range " Chuck Lever
2026-10-22 13:53 ` [PATCH v1 03/17] nfs_common: Export posix_acl_from_nfsacl for server ACL codecs Chuck Lever
2026-10-22 13:53 ` [PATCH v1 04/17] NFSD: Remove the unused NFSACL_MAXWORDS macro Chuck Lever
2026-10-22 13:53 ` [PATCH v1 05/17] NFSD: Move the definition of NFS_ACL_MAX_ENTRIES Chuck Lever
2026-10-22 13:53 ` [PATCH v1 06/17] Documentation: Add the RPC language description of NFS_ACLv2 Chuck Lever
2026-10-22 13:53 ` [PATCH v1 07/17] NFSD: Add infrastructure for generating NFS_ACLv2 XDR encoders and decoders Chuck Lever
2026-10-22 13:53 ` [PATCH v1 08/17] NFSD: Remove '#include "xdr3.h"' from nfs2acl.c Chuck Lever
2026-10-22 13:53 ` [PATCH v1 09/17] NFSD: Remove '#include "xdr.h"' " Chuck Lever
2026-10-22 13:53 ` [PATCH v1 10/17] NFSD: Switch to using the NFS_ACLv2 status codes Chuck Lever
2026-10-22 13:53 ` [PATCH v1 11/17] NFSD: Use xdrgen XDR functions for the NFS-ACL v2 NULL procedure Chuck Lever
2026-10-22 13:53 ` [PATCH v1 12/17] NFSD: Implement NFS-ACL v2 ACL encoding and decoding with xdrgen Chuck Lever
2026-10-22 13:53 ` [PATCH v1 13/17] NFSD: Use xdrgen XDR functions for the NFS-ACL v2 GETACL procedure Chuck Lever
2026-10-22 13:53 ` Chuck Lever [this message]
2026-10-22 13:53 ` [PATCH v1 15/17] NFSD: Use xdrgen XDR functions for the NFS-ACL v2 GETATTR procedure Chuck Lever
2026-10-22 13:53 ` [PATCH v1 16/17] NFSD: Use xdrgen XDR functions for the NFS-ACL v2 ACCESS procedure Chuck Lever
2026-10-22 13:53 ` [PATCH v1 17/17] NFSD: Map every NFS_ACLv2 reply status into aclstat2 Chuck Lever

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261022135341.14265-15-cel@kernel.org \
    --to=cel@kernel.org \
    --cc=dai.ngo@oracle.com \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox