[PATCH v1 0/3] Update suppattr_exclcreat bitmask

[PATCH v1 0/3] Update suppattr_exclcreat bitmask

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

While working on some new pynfs tests to verify the behavior of
OPEN(create), I found a couple of bugs in NFSD's suppattr_exclcreat
bitmask.

I've split this into two "fix" patches, because the fixes have
different LTS blast radii. The third patch is a clean-up that
should not be backported.

I will post the new pynfs tests under separate cover.

Chuck Lever (3):
  NFSD: Clear SECLABEL in the suppattr_exclcreat bitmap
  NFSD: Clear TIME_DELEG in the suppattr_exclcreat bitmap
  NFSD: Clean up nfsd4_check_open_attributes()

 fs/nfsd/nfs4proc.c | 40 +++++++++++++++++++++-------------------
 fs/nfsd/nfs4xdr.c  |  7 +++++++
 2 files changed, 28 insertions(+), 19 deletions(-)

-- 
2.51.0

[PATCH v1 1/3] NFSD: Clear SECLABEL in the suppattr_exclcreat bitmap

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

From RFC 8881:

5.8.1.14. Attribute 75: suppattr_exclcreat

> The bit vector that would set all REQUIRED and RECOMMENDED
> attributes that are supported by the EXCLUSIVE4_1 method of file
> creation via the OPEN operation. The scope of this attribute
> applies to all objects with a matching fsid.

There's nothing in RFC 8881 that states that suppattr_exclcreat is
or is not allowed to contain bits for attributes that are clear in
the reported supported_attrs bitmask. But it doesn't make sense for
an NFS server to indicate that it /doesn't/ implement an attribute,
but then also indicate that clients /are/ allowed to set that
attribute using OPEN(create) with EXCLUSIVE4_1.

Ensure that the SECURITY_LABEL and ACL bits are not set in the
suppattr_exclcreat bitmask when they are also not set in the
supported_attrs bitmask.

Fixes: 8c18f2052e75 ("nfsd41: SUPPATTR_EXCLCREAT attribute")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4xdr.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 30ce5851fe4c..51ef97c25456 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3375,6 +3375,11 @@ static __be32 nfsd4_encode_fattr4_suppattr_exclcreat(struct xdr_stream *xdr,
 	u32 supp[3];
 
 	memcpy(supp, nfsd_suppattrs[resp->cstate.minorversion], sizeof(supp));
+	if (!IS_POSIXACL(d_inode(args->dentry)))
+		supp[0] &= ~FATTR4_WORD0_ACL;
+	if (!args->contextsupport)
+		supp[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
+
 	supp[0] &= NFSD_SUPPATTR_EXCLCREAT_WORD0;
 	supp[1] &= NFSD_SUPPATTR_EXCLCREAT_WORD1;
 	supp[2] &= NFSD_SUPPATTR_EXCLCREAT_WORD2;
-- 
2.51.0

[PATCH v1 2/3] NFSD: Clear TIME_DELEG in the suppattr_exclcreat bitmap

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

From RFC 8881:

5.8.1.14. Attribute 75: suppattr_exclcreat

> The bit vector that would set all REQUIRED and RECOMMENDED
> attributes that are supported by the EXCLUSIVE4_1 method of file
> creation via the OPEN operation. The scope of this attribute
> applies to all objects with a matching fsid.

There's nothing in RFC 8881 that states that suppattr_exclcreat is
or is not allowed to contain bits for attributes that are clear in
the reported supported_attrs bitmask. But it doesn't make sense for
an NFS server to indicate that it /doesn't/ implement an attribute,
but then also indicate that clients /are/ allowed to set that
attribute using OPEN(create) with EXCLUSIVE4_1.

The FATTR4_WORD2_TIME_DELEG attributes are also not to be allowed
for OPEN(create) with EXCLUSIVE4_1. It doesn't make sense to set
a delegated timestamp on a new file.

Fixes: 7e13f4f8d27d ("nfsd: handle delegated timestamps in SETATTR")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4xdr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 51ef97c25456..8f12dee4c3b5 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3379,6 +3379,8 @@ static __be32 nfsd4_encode_fattr4_suppattr_exclcreat(struct xdr_stream *xdr,
 		supp[0] &= ~FATTR4_WORD0_ACL;
 	if (!args->contextsupport)
 		supp[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
+	supp[2] &= ~FATTR4_WORD2_TIME_DELEG_ACCESS;
+	supp[2] &= ~FATTR4_WORD2_TIME_DELEG_MODIFY;
 
 	supp[0] &= NFSD_SUPPATTR_EXCLCREAT_WORD0;
 	supp[1] &= NFSD_SUPPATTR_EXCLCREAT_WORD1;
-- 
2.51.0

Re: [PATCH v1 2/3] NFSD: Clear TIME_DELEG in the suppattr_exclcreat bitmap

[Jeff Layton]

On Fri, 2025-11-14 at 15:29 -0500, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> From RFC 8881:
> 
> 5.8.1.14. Attribute 75: suppattr_exclcreat
> 
> > The bit vector that would set all REQUIRED and RECOMMENDED
> > attributes that are supported by the EXCLUSIVE4_1 method of file
> > creation via the OPEN operation. The scope of this attribute
> > applies to all objects with a matching fsid.
> 
> There's nothing in RFC 8881 that states that suppattr_exclcreat is
> or is not allowed to contain bits for attributes that are clear in
> the reported supported_attrs bitmask. But it doesn't make sense for
> an NFS server to indicate that it /doesn't/ implement an attribute,
> but then also indicate that clients /are/ allowed to set that
> attribute using OPEN(create) with EXCLUSIVE4_1.
> 
> The FATTR4_WORD2_TIME_DELEG attributes are also not to be allowed
> for OPEN(create) with EXCLUSIVE4_1. It doesn't make sense to set
> a delegated timestamp on a new file.
> 
> Fixes: 7e13f4f8d27d ("nfsd: handle delegated timestamps in SETATTR")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/nfs4xdr.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 51ef97c25456..8f12dee4c3b5 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -3379,6 +3379,8 @@ static __be32 nfsd4_encode_fattr4_suppattr_exclcreat(struct xdr_stream *xdr,
>  		supp[0] &= ~FATTR4_WORD0_ACL;
>  	if (!args->contextsupport)
>  		supp[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
> +	supp[2] &= ~FATTR4_WORD2_TIME_DELEG_ACCESS;
> +	supp[2] &= ~FATTR4_WORD2_TIME_DELEG_MODIFY;
>  

Why not just clear those bits in NFSD_SUPPATTR_EXCLCREAT_WORD2 ?

>  	supp[0] &= NFSD_SUPPATTR_EXCLCREAT_WORD0;
>  	supp[1] &= NFSD_SUPPATTR_EXCLCREAT_WORD1;


-- 
Jeff Layton <jlayton@kernel.org>

[PATCH v1 3/3] NFSD: Clean up nfsd4_check_open_attributes()

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

The @rqstp parameter was introduced in commit 3c8e03166ae2 ("NFSv4:
do exact check about attribute specified") but has never been used.

Reduce indentation in callers to improve readability.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4proc.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 7f7e6bb23a90..dcad50846a97 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -81,8 +81,8 @@ static u32 nfsd41_ex_attrmask[] = {
 };
 
 static __be32
-check_attr_support(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
-		   u32 *bmval, u32 *writable)
+check_attr_support(struct nfsd4_compound_state *cstate, u32 *bmval,
+		   u32 *writable)
 {
 	struct dentry *dentry = cstate->current_fh.fh_dentry;
 	struct svc_export *exp = cstate->current_fh.fh_export;
@@ -103,21 +103,25 @@ check_attr_support(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 }
 
 static __be32
-nfsd4_check_open_attributes(struct svc_rqst *rqstp,
-	struct nfsd4_compound_state *cstate, struct nfsd4_open *open)
+nfsd4_check_open_attributes(struct nfsd4_compound_state *cstate,
+			    struct nfsd4_open *open)
 {
 	__be32 status = nfs_ok;
 
-	if (open->op_create == NFS4_OPEN_CREATE) {
-		if (open->op_createmode == NFS4_CREATE_UNCHECKED
-		    || open->op_createmode == NFS4_CREATE_GUARDED)
-			status = check_attr_support(rqstp, cstate,
-					open->op_bmval, nfsd_attrmask);
-		else if (open->op_createmode == NFS4_CREATE_EXCLUSIVE4_1)
-			status = check_attr_support(rqstp, cstate,
-					open->op_bmval, nfsd41_ex_attrmask);
-	}
+	if (open->op_create != NFS4_OPEN_CREATE)
+		return status;
 
+	switch (open->op_createmode) {
+	case NFS4_CREATE_UNCHECKED:
+	case NFS4_CREATE_GUARDED:
+		status = check_attr_support(cstate, open->op_bmval,
+					    nfsd_attrmask);
+		break;
+	case NFS4_CREATE_EXCLUSIVE4_1:
+		status = check_attr_support(cstate, open->op_bmval,
+					    nfsd41_ex_attrmask);
+		break;
+	}
 	return status;
 }
 
@@ -583,7 +587,7 @@ nfsd4_open(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		goto out;
 	}
 
-	status = nfsd4_check_open_attributes(rqstp, cstate, open);
+	status = nfsd4_check_open_attributes(cstate, open);
 	if (status)
 		goto out;
 
@@ -795,8 +799,7 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	if (status)
 		return status;
 
-	status = check_attr_support(rqstp, cstate, create->cr_bmval,
-				    nfsd_attrmask);
+	status = check_attr_support(cstate, create->cr_bmval, nfsd_attrmask);
 	if (status)
 		return status;
 
@@ -1216,8 +1219,7 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		return nfserrno(err);
 	status = nfs_ok;
 
-	status = check_attr_support(rqstp, cstate, setattr->sa_bmval,
-				    nfsd_attrmask);
+	status = check_attr_support(cstate, setattr->sa_bmval, nfsd_attrmask);
 	if (status)
 		goto out;
 
@@ -2270,7 +2272,7 @@ _nfsd4_verify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	if (status)
 		return status;
 
-	status = check_attr_support(rqstp, cstate, verify->ve_bmval, NULL);
+	status = check_attr_support(cstate, verify->ve_bmval, NULL);
 	if (status)
 		return status;
 
-- 
2.51.0