linux-kernel - [PATCH 2/2] vfs: fix delegated timestamp handling in setattr

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-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250722-nfsd-testing-v1-2-31321c7fc97f@kernel.org>
Date: Tue, 22 Jul 2025 14:52:28 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Alexander Viro <viro@...iv.linux.org.uk>, 
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, 
 Steven Rostedt <rostedt@...dmis.org>, 
 Masami Hiramatsu <mhiramat@...nel.org>, 
 Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, 
 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-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
 linux-trace-kernel@...r.kernel.org, linux-nfs@...r.kernel.org, 
 Jeff Layton <jlayton@...nel.org>
Subject: [PATCH 2/2] vfs: fix delegated timestamp handling in
 setattr_copy()

There are a couple of problems with delegated timestamp updates via
setattr:

1/ the ia_ctime is always clobbered by notify_change(), so setting the
ia_ctime to the same value as the ia_mtime in nfsd4_decode_fattr4()
doesn't work.

2/ while it does test the ctime's validity vs. the existing ctime and
current_time(), the same is not done for the atime or mtime. The spec
requires this.

Add a new setattr_copy_delegts() function that handles updating the
timestamps whenever ATTR_DELEG is set. For both atime and mtime,
validate and clamp the value to current_time(), and then set it. If the
mtime gets updated, also update the ctime.

Fixes: 7f2c86cba3c5 ("fs: handle delegated timestamps in setattr_copy_mgtime")
Signed-off-by: Jeff Layton <jlayton@...nel.org>
---
 fs/attr.c         | 52 ++++++++++++++++++++++++++++++++++++++--------------
 fs/nfsd/nfs4xdr.c |  4 +---
 2 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/fs/attr.c b/fs/attr.c
index 9caf63d20d03e86c535e9c8c91d49c2a34d34b7a..3e636943d26a36aeeed0ff8b428b6dd3e63f8dde 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -287,14 +287,7 @@ static void setattr_copy_mgtime(struct inode *inode, const struct iattr *attr)
 	struct timespec64 now;
 
 	if (ia_valid & ATTR_CTIME) {
-		/*
-		 * In the case of an update for a write delegation, we must respect
-		 * the value in ia_ctime and not use the current time.
-		 */
-		if (ia_valid & ATTR_DELEG)
-			now = inode_set_ctime_deleg(inode, attr->ia_ctime);
-		else
-			now = inode_set_ctime_current(inode);
+		now = inode_set_ctime_current(inode);
 	} else {
 		/* If ATTR_CTIME isn't set, then ATTR_MTIME shouldn't be either. */
 		WARN_ON_ONCE(ia_valid & ATTR_MTIME);
@@ -312,6 +305,39 @@ static void setattr_copy_mgtime(struct inode *inode, const struct iattr *attr)
 		inode_set_mtime_to_ts(inode, now);
 }
 
+/*
+ * Skip update if new value is older than existing time. Clamp
+ * to current_time() if it's in the future.
+ */
+static void setattr_copy_delegts(struct inode *inode, const struct iattr *attr)
+{
+	struct timespec64 now = current_time(inode);
+	unsigned int ia_valid = attr->ia_valid;
+
+	if (ia_valid & ATTR_MTIME) {
+		struct timespec64 cur = inode_get_mtime(inode);
+
+		if (timespec64_compare(&attr->ia_mtime, &cur) > 0) {
+			if (timespec64_compare(&attr->ia_mtime, &now) > 0)
+				inode_set_mtime_to_ts(inode, now);
+			else
+				inode_set_mtime_to_ts(inode, attr->ia_mtime);
+			inode_set_ctime_deleg(inode, attr->ia_mtime);
+		}
+	}
+
+	if (ia_valid & ATTR_ATIME) {
+		struct timespec64 cur = inode_get_atime(inode);
+
+		if (timespec64_compare(&attr->ia_atime, &cur) > 0) {
+			if (timespec64_compare(&attr->ia_atime, &now) > 0)
+				inode_set_atime_to_ts(inode, now);
+			else
+				inode_set_atime_to_ts(inode, attr->ia_atime);
+		}
+	}
+}
+
 /**
  * setattr_copy - copy simple metadata updates into the generic inode
  * @idmap:	idmap of the mount the inode was found from
@@ -352,6 +378,8 @@ void setattr_copy(struct mnt_idmap *idmap, struct inode *inode,
 		inode->i_mode = mode;
 	}
 
+	if (ia_valid & ATTR_DELEG)
+		return setattr_copy_delegts(inode, attr);
 	if (is_mgtime(inode))
 		return setattr_copy_mgtime(inode, attr);
 
@@ -359,12 +387,8 @@ void setattr_copy(struct mnt_idmap *idmap, struct inode *inode,
 		inode_set_atime_to_ts(inode, attr->ia_atime);
 	if (ia_valid & ATTR_MTIME)
 		inode_set_mtime_to_ts(inode, attr->ia_mtime);
-	if (ia_valid & ATTR_CTIME) {
-		if (ia_valid & ATTR_DELEG)
-			inode_set_ctime_deleg(inode, attr->ia_ctime);
-		else
-			inode_set_ctime_to_ts(inode, attr->ia_ctime);
-	}
+	if (ia_valid & ATTR_CTIME)
+		inode_set_ctime_to_ts(inode, attr->ia_ctime);
 }
 EXPORT_SYMBOL(setattr_copy);
 
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 8b68f74a8cf08c6aa1305a2a3093467656085e4a..e6899a3502332d686138abee2284c87fc7fbc0ae 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -537,9 +537,7 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen,
 			return nfserr_bad_xdr;
 		iattr->ia_mtime.tv_sec = modify.seconds;
 		iattr->ia_mtime.tv_nsec = modify.nseconds;
-		iattr->ia_ctime.tv_sec = modify.seconds;
-		iattr->ia_ctime.tv_nsec = modify.seconds;
-		iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET | ATTR_DELEG;
+		iattr->ia_valid |= ATTR_MTIME | ATTR_MTIME_SET | ATTR_DELEG;
 	}
 
 	/* request sanity: did attrlist4 contain the expected number of words? */

-- 
2.50.1