lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250715-tsfix-v1-1-da21665d4626@kernel.org>
Date: Tue, 15 Jul 2025 07:34:10 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Chuck Lever <chuck.lever@...cle.com>, NeilBrown <neil@...wn.name>, 
 Olga Kornievskaia <okorniev@...hat.com>, Dai Ngo <Dai.Ngo@...cle.com>, 
 Tom Talpey <tom@...pey.com>
Cc: linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org, 
 Jeff Layton <jlayton@...nel.org>
Subject: [PATCH] nfsd: don't set the ctime on delegated atime updates

Clients will typically precede a DELEGRETURN for a delegation with
delegated timestamp with a SETATTR to set the timestamps on the server
to match what the client has.

knfsd implements this by using the nfsd_setattr() infrastructure, which
will set ATTR_CTIME on any update that goes to notify_change(). This is
problematic as it means that the client will get a spurious ctime
updates when updating the atime.

Fix this by pushing the handling of ATTR_CTIME down into the decoder
functions so that they are set earlier and more deliberately, and stop
nfsd_setattr() from implicitly adding it to every notify_change() call.

Fixes: 7e13f4f8d27d ("nfsd: handle delegated timestamps in SETATTR")
Signed-off-by: Jeff Layton <jlayton@...nel.org>
---
I finally was able to spend some time on the gitr failures with
delegated timestamps and found this. With this patch I was able to run
the gitr suite in the "stress" configuration under kdevops and it
passed.
---
 fs/nfsd/nfs3xdr.c   |  3 +++
 fs/nfsd/nfs4state.c |  2 +-
 fs/nfsd/nfs4xdr.c   | 18 +++++++++---------
 fs/nfsd/nfsxdr.c    |  3 +++
 fs/nfsd/vfs.c       |  1 -
 5 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index ef4971d71ac4bfee5171537eda80dec6e367060d..3058c73fe2d204131c33e31e76f7ca15c2545d36 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -289,6 +289,9 @@ svcxdr_decode_sattr3(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 		return false;
 	}
 
+	if (iap->ia_valid)
+		iap->ia_valid |= ATTR_CTIME;
+
 	return true;
 }
 
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d5694987f86fadab985e55cce6261ad680e83b69..3aa430c8d941570840fc482efc750daa5df3ae84 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5690,7 +5690,7 @@ nfsd4_truncate(struct svc_rqst *rqstp, struct svc_fh *fh,
 		struct nfsd4_open *open)
 {
 	struct iattr iattr = {
-		.ia_valid = ATTR_SIZE,
+		.ia_valid = ATTR_SIZE | ATTR_CTIME,
 		.ia_size = 0,
 	};
 	struct nfsd_attrs attrs = {
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 3afcdbed6e1465929ec7da67593243a8bf97b3f4..568b30619c79a857429113015b2ff7081557681f 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -409,7 +409,7 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
 		if (xdr_stream_decode_u64(argp->xdr, &size) < 0)
 			return nfserr_bad_xdr;
 		iattr->ia_size = size;
-		iattr->ia_valid |= ATTR_SIZE;
+		iattr->ia_valid |= ATTR_SIZE | ATTR_CTIME;
 	}
 	if (bmval[0] & FATTR4_WORD0_ACL) {
 		status = nfsd4_decode_acl(argp, acl);
@@ -424,7 +424,7 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
 			return nfserr_bad_xdr;
 		iattr->ia_mode = mode;
 		iattr->ia_mode &= (S_IFMT | S_IALLUGO);
-		iattr->ia_valid |= ATTR_MODE;
+		iattr->ia_valid |= ATTR_MODE | ATTR_CTIME;
 	}
 	if (bmval[1] & FATTR4_WORD1_OWNER) {
 		u32 length;
@@ -438,7 +438,7 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
 					      &iattr->ia_uid);
 		if (status)
 			return status;
-		iattr->ia_valid |= ATTR_UID;
+		iattr->ia_valid |= ATTR_UID | ATTR_CTIME;
 	}
 	if (bmval[1] & FATTR4_WORD1_OWNER_GROUP) {
 		u32 length;
@@ -452,7 +452,7 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
 					      &iattr->ia_gid);
 		if (status)
 			return status;
-		iattr->ia_valid |= ATTR_GID;
+		iattr->ia_valid |= ATTR_GID | ATTR_CTIME;
 	}
 	if (bmval[1] & FATTR4_WORD1_TIME_ACCESS_SET) {
 		u32 set_it;
@@ -464,10 +464,10 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
 			status = nfsd4_decode_nfstime4(argp, &iattr->ia_atime);
 			if (status)
 				return status;
-			iattr->ia_valid |= (ATTR_ATIME | ATTR_ATIME_SET);
+			iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET | ATTR_CTIME;
 			break;
 		case NFS4_SET_TO_SERVER_TIME:
-			iattr->ia_valid |= ATTR_ATIME;
+			iattr->ia_valid |= ATTR_ATIME | ATTR_CTIME;
 			break;
 		default:
 			return nfserr_bad_xdr;
@@ -492,10 +492,10 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
 			status = nfsd4_decode_nfstime4(argp, &iattr->ia_mtime);
 			if (status)
 				return status;
-			iattr->ia_valid |= (ATTR_MTIME | ATTR_MTIME_SET);
+			iattr->ia_valid |= ATTR_MTIME | ATTR_MTIME_SET | ATTR_CTIME;
 			break;
 		case NFS4_SET_TO_SERVER_TIME:
-			iattr->ia_valid |= ATTR_MTIME;
+			iattr->ia_valid |= ATTR_MTIME | ATTR_CTIME;
 			break;
 		default:
 			return nfserr_bad_xdr;
@@ -519,7 +519,7 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
 		if (xdr_stream_decode_u32(argp->xdr, &mask) < 0)
 			return nfserr_bad_xdr;
 		*umask = mask & S_IRWXUGO;
-		iattr->ia_valid |= ATTR_MODE;
+		iattr->ia_valid |= ATTR_MODE | ATTR_CTIME;
 	}
 	if (bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
 		fattr4_time_deleg_access access;
diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
index fc262ceafca972df353a389eea2d7e99fd6bc1d2..8372a374623a5d98fff7c0227a73a9b562da407d 100644
--- a/fs/nfsd/nfsxdr.c
+++ b/fs/nfsd/nfsxdr.c
@@ -196,6 +196,9 @@ svcxdr_decode_sattr(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 			iap->ia_valid &= ~(ATTR_ATIME_SET|ATTR_MTIME_SET);
 	}
 
+	if (iap->ia_valid)
+		iap->ia_valid |= ATTR_CTIME;
+
 	return true;
 }
 
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index cd689df2ca5d7396cffb5ed9dc14f774a8f3881c..383a724e9aa20a4c6a71a563281fc8a618dd36d3 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -470,7 +470,6 @@ static int __nfsd_setattr(struct dentry *dentry, struct iattr *iap)
 	if (!iap->ia_valid)
 		return 0;
 
-	iap->ia_valid |= ATTR_CTIME;
 	return notify_change(&nop_mnt_idmap, dentry, iap, NULL);
 }
 

---
base-commit: 347e9f5043c89695b01e66b3ed111755afcf1911
change-id: 20250715-tsfix-780246904a01

Best regards,
-- 
Jeff Layton <jlayton@...nel.org>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ