[PATCH v2 2/3] nfsd: accept a backdated timestamp from a delegation holder

[Jeff Layton <jlayton@kernel.org>]

A client with an attribute delegation reports the file times in a
SETATTR at DELEGRETURN. nfsd ignores a time that moves backwards. If the
file was written, nfsd4_finalize_deleg_timestamps() then stamps the
c/mtime with the current time, so the file keeps the DELEGRETURN time.
cp -p, rsync -t and tar -x lose timestamps. The SETATTR returns NFS4_OK.

The client applies an explicit utimensat() to its own inode. It sends no
SETATTR while it holds the delegation, so the backdated value reaches
nfsd only as TIME_DELEG_MODIFY. The client can send an RPC for each time
change instead. That also works, but it loses the caching that the
delegation allows.

The delegation makes the client the authority for these times, so treat
its SETATTR as a statement of fact. The client can set the same value
with an ordinary SETATTR, which nfsd applies without a check.

- nfsd accepts a backwards atime or mtime.
- An mtime that moves backwards sets the ctime to the current time.
  inode_set_ctime_deleg() does that, so nfsd does not compare against the
  ctime here. The ctime never moves backwards.
- The client reports the times at every DELEGRETURN, changed or not. Drop
  a report that already matches the inode, and leave the ctime alone when
  neither the mtime nor the data moved. Otherwise an untouched file gets a
  new change attribute every time a delegation comes back and every other
  client drops its cache, notify_change() runs for nothing, and a holder
  that does not own the file gets -EPERM out of setattr_prepare() where it
  used to get NFS4_OK.
- dl_setattr stops nfsd4_finalize_deleg_timestamps() from stamping over
  the reported times, so only the branch that carries a c/mtime update may
  set it. A write that follows a no-op SETATTR would otherwise lose its
  timestamps.
- nfsd still clamps a future time.
- The CB_GETATTR path keeps the old rule. A backwards time there shows
  a stale report.

RFC 9754 says that the server ignores a time before the original time.
This patch does not follow that sentence. The same section also says
that the server MUST accept the change or MUST reject it with
NFS4ERR_DELAY. A silent discard does neither. A retry after
NFS4ERR_DELAY carries the same backdated value, so that option cannot
succeed.

There is still one gap: nfsd cannot tell an explicit utimensat() from a
report of a write. An mtime after the ctime and before the current time
therefore sets the ctime to that mtime, instead of to "now". RFC 9754
requires this. Fixing that would require the client to issue an RPC for
the mtime.

Fixes: 3952f1cbcbc4 ("nfsd: fix SETATTR updates for delegated timestamps")
Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4proc.c | 63 +++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 51 insertions(+), 12 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index bb74eef43938..36fead027fce 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1292,27 +1292,66 @@ nfsd4_secinfo_no_name_release(union nfsd4_op_u *u)
 }
 
 /*
- * Validate that the requested timestamps are within the acceptable range. If
- * timestamp appears to be in the future, then it will be clamped to
- * current_time().
+ * A client holding a delegation with delegated timestamps is the authority for
+ * the file's timestamps, so a SETATTR from it asserts what they are rather than
+ * reporting that they have advanced. Honor a value that moves a timestamp
+ * backwards: the client could set the same value with an ordinary SETATTR, so
+ * refusing it here only loses data. Clamp a value in the future to the current
+ * time, as RFC 9754 permits.
+ */
+static void
+clamp_deleg_time(struct timespec64 *req, const struct timespec64 *now)
+{
+	if (timespec64_compare(req, now) > 0)
+		*req = *now;
+}
+
+/*
+ * Apply the timestamps that a delegation holder supplied in a SETATTR.
  */
 static void
 vet_deleg_attrs(struct nfsd4_setattr *setattr, struct nfs4_delegation *dp)
 {
-	struct timespec64 now = current_time(dp->dl_stid.sc_file->fi_inode);
+	struct inode *inode = dp->dl_stid.sc_file->fi_inode;
+	struct timespec64 now = current_time(inode);
 	struct iattr *iattr = &setattr->sa_iattr;
 
-	if ((setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) &&
-	    !nfsd4_vet_deleg_time(&iattr->ia_atime, &dp->dl_atime, &now))
-		iattr->ia_valid &= ~(ATTR_ATIME | ATTR_ATIME_SET);
+	/*
+	 * The client reports the times at every DELEGRETURN, changed or not.
+	 * Drop a report that matches the inode. An untouched file then keeps its
+	 * change attribute, and nfsd_setattr() skips the call into the
+	 * filesystem.
+	 *
+	 * The times are read without i_rwsem. A conflicting writer must break
+	 * the delegation first, and FMODE_NOCMTIME stops the holder's own writes
+	 * from stamping the c/mtime. touch_atime() and a CB_GETATTR can still
+	 * move them here. A stale read then costs at most one extra update.
+	 */
+	if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_ACCESS) {
+		struct timespec64 atime = inode_get_atime(inode);
+
+		clamp_deleg_time(&iattr->ia_atime, &now);
+
+		if (timespec64_equal(&iattr->ia_atime, &atime))
+			iattr->ia_valid &= ~(ATTR_ATIME | ATTR_ATIME_SET);
+	}
 
 	if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
-		if (nfsd4_vet_deleg_time(&iattr->ia_mtime, &dp->dl_mtime, &now)) {
+		struct timespec64 mtime = inode_get_mtime(inode);
+
+		clamp_deleg_time(&iattr->ia_mtime, &now);
+
+		if (dp->dl_written ||
+		    !timespec64_equal(&iattr->ia_mtime, &mtime)) {
 			iattr->ia_ctime = iattr->ia_mtime;
-			if (nfsd4_vet_deleg_time(&iattr->ia_ctime, &dp->dl_ctime, &now))
-				dp->dl_setattr = true;
-			else
-				iattr->ia_valid &= ~(ATTR_CTIME | ATTR_CTIME_SET);
+
+			/*
+			 * Keep nfsd4_finalize_deleg_timestamps() from stamping
+			 * over the values the client just supplied. Only the
+			 * branch that carries a c/mtime update may set this, or
+			 * a write after a no-op SETATTR loses its timestamps.
+			 */
+			dp->dl_setattr = true;
 		} else {
 			iattr->ia_valid &= ~(ATTR_CTIME | ATTR_CTIME_SET |
 					     ATTR_MTIME | ATTR_MTIME_SET);

-- 
2.55.0