linux-kernel - [PATCH v2] nfsd: don't set the ctime on delegated atime updates

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250716-tsfix-v2-1-b834899cf4d0@kernel.org>
Date: Wed, 16 Jul 2025 09:34:29 -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 v2] 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.

POSIX unfortunately doesn't phrase it succinctly, but updating the atime
due to reads should not update the ctime. In this case, the client is
sending a SETATTR to update the atime on the server to match its latest
value. The ctime should not be advanced in this case as that would
incorrectly indicate a change to the inode.

Fix this by not implicitly setting ATTR_CTIME when ATTR_DELEG is set in
__nfsd_setattr(). The decoder for FATTR4_WORD2_TIME_DELEG_MODIFY already
sets ATTR_CTIME, so this is sufficient to make it skip setting the ctime
on atime-only updates.

Fixes: 7e13f4f8d27d ("nfsd: handle delegated timestamps in SETATTR")
Signed-off-by: Jeff Layton <jlayton@...nel.org>
---
I had missed a place where ATTR_CTIME needed to be set in the previous
patch. I think this might be a better approach.

Note that I do still see the occasional test failure due to a client bug
with delegated timestamps, so this isn't enough on its own to enable
them by default. Stay tuned!
---
Changes in v2:
- only implicitly set ATTR_CTIME if ATTR_DELEG isn't set
- Link to v1: https://lore.kernel.org/r/20250715-tsfix-v1-1-da21665d4626@kernel.org
---
 fs/nfsd/vfs.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index ee78b6fb17098b788b07f5cd90598e678244b57e..c25e43dfa4aa4e3f730ffbb93f6b981d90c19c4c 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -470,7 +470,15 @@ static int __nfsd_setattr(struct dentry *dentry, struct iattr *iap)
 	if (!iap->ia_valid)
 		return 0;

-	iap->ia_valid |= ATTR_CTIME;
+	/*
+	 * If ATTR_DELEG is set, then this an update from a client that holds
+	 * a delegation. If this is only an update for the atime, the ctime should
+	 * not be changed. If the update contains the mtime too, then ATTR_CTIME
+	 * should already be set.
+	 */
+	if (!(iap->ia_valid & ATTR_DELEG))
+		iap->ia_valid |= ATTR_CTIME;
+
 	return notify_change(&nop_mnt_idmap, dentry, iap, NULL);
 }

---
base-commit: 7351f1092b3cde79f654dc49a82d51c7ada0a1f2
change-id: 20250715-tsfix-780246904a01

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