[PATCH v3 0/8] nfsd: more delegated timestamp fixes

[PATCH v3 0/8] nfsd: more delegated timestamp fixes

[Jeff Layton]

This version fixes the existing patches and adds some new ones to fix
problems that agentic review spotted. Note that there is also a
companion client-side series that fixes some related issues.

The catch with this version is that we need a fix to
inode_set_ctime_deleg(), and that lives in fs/inode.c. We'll either need
to get Christian to take that one or get his blessing to merge it via
the nfsd tree. It's a pretty small patch, so the latter might be OK.

Original cover letter follows:

Chuck recently added a new DELEG27 pynfs test that fails against nfsd.
These patches fix the behavior of delegated timestamps (again), such
that DELEG27 now passes.

I also ran some tests with the git regression testsuite which turns out
to be very sensitive to timestamp bugs, and that turned up a few more
server-side bugs.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v3:
- "Honor delegated timestamps when change and size match":
  - record applied times from SETATTR too, so a stale CB_GETATTR reply
    can't move them backward
  - record ctime from the inode; atime/mtime from the untruncated values
  - move the vet/record prototypes to nfs4ctl.h
- "don't stamp over an applied delegated mtime at DELEGRETURN":
  - share the vetting between SETATTR and CB_GETATTR
  - set dl_setattr only after the holder's mtime is applied
  - test dl_setattr under inode_lock at DELEGRETURN
- "don't vet delegated times against an exclusive-create verifier":
  also set the floors for read timestamp delegations
- "mark the delegation written on COPY/CLONE instead of stamping c/mtime":
  - look up the nfs4_file for every async COPY
  - remove the now-unused nfsd_update_cmtime_attr()
- New: fs: check a fine-grained time before clamping a delegated ctime
- New: nfsd: check a fine-grained time before clamping a delegated mtime
- New: nfsd: raise delegated time floors after a size-changing SETATTR
- Kernel-doc, comment and commit message fixes
- Link to v2: https://lore.kernel.org/r/20261106-deleg-cb-getattr-times-v2-0-8433618ab9a1@kernel.org

Changes in v2:
- "Honor delegated timestamps when change and size match" reworked:
  - set ATTR_FORCE when applying the holder's times
  - record applied times in dl_[amc]time; vet and record under inode_lock
  - vet SETATTR's delegated times in nfsd_setattr() under inode_lock
  - also mark file modified on a holder WRITE whose mtime didn't pass dl_mtime
  - always report ncf_initial_cinfo as the change attr for timestamp delegations
  - catch the holder's own server-side changes (SETATTR, xattrs, etc.)
    via a change attribute saved at grant
  - log, rather than fail the GETATTR, if applying the holder's times fails
- New: don't stamp over an applied delegated mtime at DELEGRETURN
- New: don't vet delegated times against an exclusive-create verifier
- New: mark the delegation written on COPY/CLONE instead of stamping c/mtime
- Link to v1: https://lore.kernel.org/r/20261102-deleg-cb-getattr-times-v1-0-ca0326d88623@kernel.org

---
Jeff Layton (8):
      nfsd: Report vetted delegated timestamps in GETATTR replies
      nfsd: Honor delegated timestamps when change and size match
      nfsd: don't stamp over an applied delegated mtime at DELEGRETURN
      nfsd: don't vet delegated times against an exclusive-create verifier
      nfsd: mark the delegation written on COPY/CLONE instead of stamping c/mtime
      fs: check a fine-grained time before clamping a delegated ctime
      nfsd: check a fine-grained time before clamping a delegated mtime
      nfsd: raise delegated time floors after a size-changing SETATTR

 fs/inode.c             |  16 ++-
 fs/nfsd/nfs4callback.c |   2 +-
 fs/nfsd/nfs4ctl.h      |  15 +++
 fs/nfsd/nfs4proc.c     | 101 +++++++++--------
 fs/nfsd/nfs4state.c    | 292 +++++++++++++++++++++++++++++++++++++------------
 fs/nfsd/nfs4xdr.c      |  10 +-
 fs/nfsd/state.h        |   6 +-
 fs/nfsd/vfs.c          |   6 +
 fs/nfsd/vfs.h          |   4 +
 fs/nfsd/xdr4.h         |   9 +-
 10 files changed, 325 insertions(+), 136 deletions(-)
---
base-commit: d692f252c52b8d1b4bc7481554f314df31ed8249
change-id: 20261102-deleg-cb-getattr-times-8aa926029b16

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

[PATCH v3 1/8] nfsd: Report vetted delegated timestamps in GETATTR replies

[Jeff Layton]

When a CB_GETATTR reply shows the file modified, nfsd4_encode_fattr4()
overrides atime and mtime with the raw time_deleg_access and
time_deleg_modify values from the reply. cb_getattr_update_times() has
already vetted those values per RFC 9754 section 5 and written the
result to the inode, so the override undoes that vetting:

- a time older than the one at delegation grant, which the server
  ignores, is still reported to the querying client
- a time in the future is reported unclamped

vfs_getattr() runs after the update and already returns the vetted
times. Drop the override.

Fixes: 6ae30d6eb26b ("nfsd: add support for delegated timestamps")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4xdr.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 90b44163f886..434b6cfc5741 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4098,14 +4098,8 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 		if (ncf->ncf_file_modified) {
 			++ncf->ncf_initial_cinfo;
 			args.stat.size = ncf->ncf_cur_fsize;
-			if (!timespec64_is_epoch(&ncf->ncf_cb_mtime))
-				args.stat.mtime = ncf->ncf_cb_mtime;
 		}
 		args.change_attr = ncf->ncf_initial_cinfo;
-
-		if (!timespec64_is_epoch(&ncf->ncf_cb_atime))
-			args.stat.atime = ncf->ncf_cb_atime;
-
 		nfs4_put_stid(&dp->dl_stid);
 	} else {
 		args.change_attr = nfsd4_change_attribute(&args.stat);

-- 
2.56.0

[PATCH v3 2/8] nfsd: Honor delegated timestamps when change and size match

[Jeff Layton]

nfsd4_deleg_getattr_conflict() applies time_deleg_access and
time_deleg_modify from a CB_GETATTR reply only when the reply's change
attribute or size differs from the server's. RFC 9754 section 5 makes
the holder of a timestamp delegation the authority for atime and mtime,
and puts no such condition on accepting them.

Because the delegation's file has FMODE_NOCMTIME, the holder's WRITEs
change neither mtime nor the change attribute on the server. After a
flushed WRITE, the CB_GETATTR reply matches on change and size, the
holder's mtime is dropped, and other clients see new data with the
pre-WRITE mtime. Since commit 304d81a2fbf2 ("nfsd: fix file change
detection in CB_GETATTR") compares against the current file size, this
also hits flushed extending WRITEs. Holder reads lose the atime the
same way.

- For timestamp delegations, always vet and apply the CB_GETATTR
  times, with ATTR_FORCE: the GETATTR caller need not own the file.
- Record applied times in dl_[amc]time, from both CB_GETATTR and
  SETATTR, so a repeated or stale holder time can't reapply or move
  them backward. Vet, apply and record under inode_lock; SETATTR's
  vetting moves into nfsd_setattr() for that.
- Mark the file modified if mtime advanced, if dl_written shows a
  holder WRITE (clock skew can keep its mtime from passing), or if the
  server's change attribute moved other than by nfsd's own timestamp
  updates: the holder's SETATTRs, xattrs, fallocates, LINKs etc.
  neither break its delegation nor show up in its CB_GETATTR reply.
  Track that in ncf_srv_cinfo, not dl_written, which would make
  DELEGRETURN stamp mtime.
- Setting atime can bump i_version, which would leak into the
  stat-based change attribute and later go backwards against
  ++ncf_initial_cinfo. For timestamp delegations, always report
  ncf_initial_cinfo instead.
- Log, rather than fail the GETATTR, if applying the times fails.

Fixes: 6ae30d6eb26b ("nfsd: add support for delegated timestamps")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4callback.c |   2 +-
 fs/nfsd/nfs4ctl.h      |  15 ++++
 fs/nfsd/nfs4proc.c     |  43 +++---------
 fs/nfsd/nfs4state.c    | 184 ++++++++++++++++++++++++++++++++++++++++++-------
 fs/nfsd/nfs4xdr.c      |   4 +-
 fs/nfsd/state.h        |   4 +-
 fs/nfsd/vfs.c          |   6 ++
 fs/nfsd/vfs.h          |   4 ++
 8 files changed, 199 insertions(+), 63 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 70ab4fd32195..901aeab88956 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -396,7 +396,7 @@ encode_cb_getattr4args(struct xdr_stream *xdr, struct nfs4_cb_compound_hdr *hdr,
 	u32 bmap[3];
 
 	bmap[0] = FATTR4_WORD0_SIZE;
-	if (!ncf->ncf_file_modified)
+	if (!READ_ONCE(ncf->ncf_file_modified))
 		bmap[0] |= FATTR4_WORD0_CHANGE;
 
 	if (deleg_attrs_deleg(dp->dl_type)) {
diff --git a/fs/nfsd/nfs4ctl.h b/fs/nfsd/nfs4ctl.h
index e0d1246ca337..bcc42df3a9ee 100644
--- a/fs/nfsd/nfs4ctl.h
+++ b/fs/nfsd/nfs4ctl.h
@@ -22,6 +22,8 @@ struct inode;
 struct dentry;
 struct svc_rqst;
 struct nfsd_net;
+struct iattr;
+struct nfs4_delegation;
 
 #ifdef CONFIG_NFSD_V4
 extern unsigned long max_delegations;
@@ -35,6 +37,10 @@ bool nfsd4_spo_must_allow(struct svc_rqst *rqstp);
 int nfsd4_create_laundry_wq(void);
 void nfsd4_destroy_laundry_wq(void);
 bool nfsd_wait_for_delegreturn(struct svc_rqst *rqstp, struct inode *inode);
+void nfsd4_vet_deleg_attrs(struct iattr *iattr, struct nfs4_delegation *dp,
+			   u32 deleg_attrs);
+void nfsd4_record_deleg_attrs(const struct iattr *iattr,
+			      struct nfs4_delegation *dp);
 
 extern int nfsd4_is_junction(struct dentry *dentry);
 extern int register_cld_notifier(void);
@@ -63,6 +69,15 @@ static inline bool nfsd_wait_for_delegreturn(struct svc_rqst *rqstp,
 {
 	return false;
 }
+static inline void nfsd4_vet_deleg_attrs(struct iattr *iattr,
+					 struct nfs4_delegation *dp,
+					 u32 deleg_attrs)
+{
+}
+static inline void nfsd4_record_deleg_attrs(const struct iattr *iattr,
+					    struct nfs4_delegation *dp)
+{
+}
 
 static inline int nfsd4_is_junction(struct dentry *dentry)
 {
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 03ea955f1fe9..4e5d5852bf57 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1338,35 +1338,6 @@ nfsd4_secinfo_no_name_release(union nfsd4_op_u *u)
 		exp_put(u->secinfo_no_name.sin_exp);
 }
 
-/*
- * 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().
- */
-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 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);
-
-	if (setattr->sa_bmval[2] & FATTR4_WORD2_TIME_DELEG_MODIFY) {
-		if (nfsd4_vet_deleg_time(&iattr->ia_mtime, &dp->dl_mtime, &now)) {
-			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);
-		} else {
-			iattr->ia_valid &= ~(ATTR_CTIME | ATTR_CTIME_SET |
-					     ATTR_MTIME | ATTR_MTIME_SET);
-		}
-	}
-}
-
 static __be32
 nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	      union nfsd4_op_u *u)
@@ -1378,10 +1349,11 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		.na_pacl	= posix_acl_dup(setattr->sa_pacl),
 		.na_dpacl	= posix_acl_dup(setattr->sa_dpacl),
 	};
-	bool save_no_wcc, deleg_attrs;
 	struct nfs4_stid *st = NULL;
 	struct inode *inode;
 	__be32 status = nfs_ok;
+	bool save_no_wcc;
+	u32 deleg_attrs;
 	int err;
 
 	deleg_attrs = setattr->sa_bmval[2] & (FATTR4_WORD2_TIME_DELEG_ACCESS |
@@ -1407,13 +1379,13 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 			/* Only for *_ATTRS_DELEG flavors */
 			if (deleg_attrs_deleg(dp->dl_type)) {
-				vet_deleg_attrs(setattr, dp);
+				/* nfsd_setattr() vets them under inode_lock */
+				attrs.na_deleg = dp;
+				attrs.na_deleg_attrs = deleg_attrs;
 				status = nfs_ok;
 			}
 		}
 	}
-	if (st)
-		nfs4_put_stid(st);
 	if (status)
 		goto out_err;
 
@@ -1452,6 +1424,8 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 out:
 	fh_drop_write(&cstate->current_fh);
 out_err:
+	if (st)
+		nfs4_put_stid(st);
 	nfsd_attrs_free(&attrs);
 	return status;
 }
@@ -1463,8 +1437,9 @@ static void nfsd4_file_mark_deleg_written(struct nfs4_file *fi)
 		struct nfs4_delegation *dp = list_first_entry(&fi->fi_delegations,
 							      struct nfs4_delegation, dl_perfile);
 
+		/* Read locklessly by nfsd4_deleg_getattr_conflict() */
 		if (dp->dl_type == OPEN_DELEGATE_WRITE_ATTRS_DELEG)
-			dp->dl_written = true;
+			WRITE_ONCE(dp->dl_written, true);
 	}
 	spin_unlock(&fi->fi_lock);
 }
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index cd70d86f4bc6..a8966d1f786f 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1472,7 +1472,7 @@ static void nfsd4_finalize_deleg_timestamps(struct nfs4_delegation *dp, struct f
 	spin_unlock(&f->f_lock);
 
 	/* was it never written? */
-	if (!dp->dl_written)
+	if (!READ_ONCE(dp->dl_written))
 		return;
 
 	/* did it get a setattr for the timestamps at some point? */
@@ -7539,6 +7539,7 @@ nfs4_open_delegation(struct svc_rqst *rqstp, struct nfsd4_open *open,
 		open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG :
 						    OPEN_DELEGATE_WRITE;
 		dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
+		dp->dl_cb_fattr.ncf_srv_cinfo = dp->dl_cb_fattr.ncf_initial_cinfo;
 		dp->dl_atime = stat.atime;
 		dp->dl_ctime = stat.ctime;
 		dp->dl_mtime = stat.mtime;
@@ -10718,16 +10719,111 @@ bool nfsd4_vet_deleg_time(struct timespec64 *req, const struct timespec64 *orig,
 	return true;
 }
 
-static int cb_getattr_update_times(struct dentry *dentry, struct nfs4_delegation *dp)
+/**
+ * nfsd4_vet_deleg_attrs - vet the delegated timestamps in a SETATTR
+ * @iattr: attributes to be set
+ * @dp: timestamp delegation held by the SETATTR's sender
+ * @deleg_attrs: FATTR4_WORD2_TIME_DELEG_* bits in the SETATTR
+ *
+ * Drop requested timestamps that don't pass nfsd4_vet_deleg_time(), and
+ * clamp ones in the future. Set dl_setattr if both the mtime and ctime
+ * pass.
+ *
+ * Context: Caller holds the inode_lock across this, applying @iattr, and
+ * nfsd4_record_deleg_attrs(), so cb_getattr_update_times() can't move
+ * dl_*time in between.
+ */
+void nfsd4_vet_deleg_attrs(struct iattr *iattr, struct nfs4_delegation *dp,
+			   u32 deleg_attrs)
+{
+	struct inode *inode = dp->dl_stid.sc_file->fi_inode;
+	struct timespec64 now = current_time(inode);
+
+	lockdep_assert_held_write(&inode->i_rwsem);
+
+	if ((deleg_attrs & FATTR4_WORD2_TIME_DELEG_ACCESS) &&
+	    !nfsd4_vet_deleg_time(&iattr->ia_atime, &dp->dl_atime, &now))
+		iattr->ia_valid &= ~(ATTR_ATIME | ATTR_ATIME_SET);
+
+	if (deleg_attrs & FATTR4_WORD2_TIME_DELEG_MODIFY) {
+		if (nfsd4_vet_deleg_time(&iattr->ia_mtime, &dp->dl_mtime, &now)) {
+			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);
+		} else {
+			iattr->ia_valid &= ~(ATTR_CTIME | ATTR_CTIME_SET |
+					     ATTR_MTIME | ATTR_MTIME_SET);
+		}
+	}
+}
+
+/**
+ * nfsd4_record_deleg_attrs - record delegated timestamps just applied
+ * @iattr: vetted attributes, not the copy notify_change() altered
+ * @dp: timestamp delegation whose holder supplied the times
+ *
+ * Set dl_*time to the times applied, so a stale holder time can't move
+ * them backward and a repeated CB_GETATTR reply isn't reapplied.
+ *
+ * Context: Caller holds the inode_lock.
+ */
+void nfsd4_record_deleg_attrs(const struct iattr *iattr,
+			      struct nfs4_delegation *dp)
+{
+	struct inode *inode = dp->dl_stid.sc_file->fi_inode;
+
+	lockdep_assert_held_write(&inode->i_rwsem);
+
+	if (iattr->ia_valid & ATTR_ATIME_SET)
+		dp->dl_atime = iattr->ia_atime;
+	if (iattr->ia_valid & ATTR_MTIME_SET)
+		dp->dl_mtime = iattr->ia_mtime;
+	/* inode_set_ctime_deleg() keeps a later ctime */
+	if (iattr->ia_valid & ATTR_CTIME)
+		dp->dl_ctime = inode_get_ctime(inode);
+}
+
+static int nfsd4_srv_cinfo(const struct path *path, u64 *cinfo)
+{
+	struct kstat stat;
+	int ret;
+
+	ret = vfs_getattr(path, &stat,
+			  STATX_MODE | STATX_CTIME | STATX_CHANGE_COOKIE,
+			  AT_STATX_SYNC_AS_STAT);
+	if (!ret)
+		*cinfo = nfsd4_change_attribute(&stat);
+	return ret;
+}
+
+static void cb_getattr_update_times(const struct path *path,
+				    struct nfs4_delegation *dp)
 {
-	struct inode *inode = d_inode(dentry);
+	struct inode *inode = d_inode(path->dentry);
 	struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
-	struct iattr attrs = { };
+	struct iattr attrs = { }, tmp;
+	u64 cinfo;
 	int ret;
 
+	/* Serialize vetting against dl_*time with their update below */
+	inode_lock(inode);
 	if (deleg_attrs_deleg(dp->dl_type)) {
 		struct timespec64 now = current_time(inode);
 
+		/*
+		 * The holder's own SETATTRs, xattr changes, fallocates, LINKs
+		 * etc. don't break its delegation and don't show up in its
+		 * CB_GETATTR reply. Any change to the server's change
+		 * attribute other than nfsd's own timestamp updates is one
+		 * of those.
+		 */
+		if (!READ_ONCE(ncf->ncf_file_modified) &&
+		    (nfsd4_srv_cinfo(path, &cinfo) ||
+		     cinfo != ncf->ncf_srv_cinfo))
+			WRITE_ONCE(ncf->ncf_file_modified, true);
+
 		attrs.ia_atime = ncf->ncf_cb_atime;
 		attrs.ia_mtime = ncf->ncf_cb_mtime;
 
@@ -10740,24 +10836,53 @@ static int cb_getattr_update_times(struct dentry *dentry, struct nfs4_delegation
 			if (nfsd4_vet_deleg_time(&attrs.ia_ctime, &dp->dl_ctime, &now))
 				attrs.ia_valid |= ATTR_CTIME | ATTR_CTIME_SET;
 		}
+
+		/*
+		 * The times are the holder's, so don't hold the GETATTR
+		 * caller to the owner check for setting them.
+		 */
+		if (attrs.ia_valid)
+			attrs.ia_valid |= ATTR_FORCE;
 	} else {
 		attrs.ia_valid |= ATTR_MTIME | ATTR_CTIME;
 	}
 
 	if (!attrs.ia_valid)
-		return 0;
+		goto out_unlock;
 
 	attrs.ia_valid |= ATTR_DELEG;
-	inode_lock(inode);
-	ret = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
+	/* notify_change() truncates the times in its iattr argument */
+	tmp = attrs;
+	ret = notify_change(&nop_mnt_idmap, path->dentry, &tmp, NULL);
+	if (ret) {
+		/* Don't fail the GETATTR over the holder's times */
+		pr_notice_ratelimited("nfsd: Unable to apply delegated timestamps on inode %02x:%02x:%llu: %d\n",
+				      MAJOR(inode->i_sb->s_dev),
+				      MINOR(inode->i_sb->s_dev),
+				      inode->i_ino, ret);
+		goto out_unlock;
+	}
+
+	if (!deleg_attrs_deleg(dp->dl_type))
+		goto out_unlock;
+
+	nfsd4_record_deleg_attrs(&attrs, dp);
+	/*
+	 * FMODE_NOCMTIME kept the holder's WRITEs from bumping the change
+	 * attribute; a newer delegated mtime is evidence of them.
+	 */
+	if (attrs.ia_valid & ATTR_MTIME_SET)
+		WRITE_ONCE(ncf->ncf_file_modified, true);
+	if (!nfsd4_srv_cinfo(path, &cinfo))
+		ncf->ncf_srv_cinfo = cinfo;
+out_unlock:
 	inode_unlock(inode);
-	return ret;
 }
 
 /**
  * nfsd4_deleg_getattr_conflict - Recall if GETATTR causes conflict
  * @rqstp: RPC transaction context
- * @dentry: dentry of inode to be checked for a conflict
+ * @path: path of inode to be checked for a conflict
  * @pdp: returned WRITE delegation, if one was found
  *
  * This function is called when there is a conflict between a write
@@ -10767,12 +10892,14 @@ static int cb_getattr_update_times(struct dentry *dentry, struct nfs4_delegation
  * delegation before replying to the GETATTR. See RFC 8881 section
  * 18.7.4.
  *
- * Returns 0 if there is no conflict; otherwise an nfs_stat
- * code is returned. If @pdp is set to a non-NULL value, then the
- * caller must put the reference.
+ * If @pdp is set to a non-NULL value, then the caller must put the
+ * reference.
+ *
+ * Return: nfs_ok if there is no conflict or a CB_GETATTR resolved it;
+ * otherwise an nfs_stat code.
  */
 __be32
-nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
+nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, const struct path *path,
 			     struct nfs4_delegation **pdp)
 {
 	struct nfsd_thread_local_info *ntli = rqstp->rq_private;
@@ -10780,7 +10907,7 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
 	struct nfs4_delegation *dp = NULL;
 	struct file_lease *fl;
 	struct nfs4_cb_fattr *ncf;
-	struct inode *inode = d_inode(dentry);
+	struct inode *inode = d_inode(path->dentry);
 	__be32 status;
 
 	ctx = locks_inode_context(inode);
@@ -10830,25 +10957,32 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct dentry *dentry,
 		status = nfs_ok;
 		goto out_status;
 	}
-	if (!ncf->ncf_file_modified) {
+	if (!READ_ONCE(ncf->ncf_file_modified)) {
 		if (ncf->ncf_initial_cinfo != ncf->ncf_cb_change)
-			ncf->ncf_file_modified = true;
+			WRITE_ONCE(ncf->ncf_file_modified, true);
 		else if (i_size_read(inode) != ncf->ncf_cb_fsize)
-			ncf->ncf_file_modified = true;
+			WRITE_ONCE(ncf->ncf_file_modified, true);
+		/* FMODE_NOCMTIME hid flushed WRITEs from the change attr */
+		else if (READ_ONCE(dp->dl_written))
+			WRITE_ONCE(ncf->ncf_file_modified, true);
 	}
-	if (ncf->ncf_file_modified) {
-		int err;
-
+	/*
+	 * The holder of a timestamp delegation is the authority for atime
+	 * and mtime (RFC 9754, section 5), even if change and size match.
+	 * Applying them can bump i_version, so for these delegations the
+	 * reply always takes its change attribute from ncf_initial_cinfo.
+	 */
+	if (READ_ONCE(ncf->ncf_file_modified) || deleg_attrs_deleg(dp->dl_type)) {
 		/*
 		 * Per section 10.4.3 of RFC 8881, the server would
 		 * not update the file's metadata with the client's
 		 * modified size
 		 */
-		err = cb_getattr_update_times(dentry, dp);
-		if (err) {
-			status = nfserrno(err);
-			goto out_status;
-		}
+		cb_getattr_update_times(path, dp);
+		/*
+		 * Set even if unmodified: another thread may set
+		 * ncf_file_modified before the caller encodes the reply.
+		 */
 		ncf->ncf_cur_fsize = ncf->ncf_cb_fsize;
 		*pdp = dp;
 		return nfs_ok;
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 434b6cfc5741..c6e0cfba07da 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4084,7 +4084,7 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 	    (attrmask[1] & (FATTR4_WORD1_TIME_ACCESS |
 			    FATTR4_WORD1_TIME_MODIFY |
 			    FATTR4_WORD1_TIME_METADATA))) {
-		status = nfsd4_deleg_getattr_conflict(rqstp, dentry, &dp);
+		status = nfsd4_deleg_getattr_conflict(rqstp, &path, &dp);
 		if (status)
 			goto out;
 	}
@@ -4095,7 +4095,7 @@ nfsd4_encode_fattr4(struct svc_rqst *rqstp, struct xdr_stream *xdr,
 	if (dp) {
 		struct nfs4_cb_fattr *ncf = &dp->dl_cb_fattr;
 
-		if (ncf->ncf_file_modified) {
+		if (READ_ONCE(ncf->ncf_file_modified)) {
 			++ncf->ncf_initial_cinfo;
 			args.stat.size = ncf->ncf_cur_fsize;
 		}
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index d9aecef95bae..9e8699810e33 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -212,6 +212,8 @@ struct nfs4_cb_fattr {
 
 	bool ncf_file_modified;
 	u64 ncf_initial_cinfo;
+	/* change attr after nfsd's last timestamp update; inode_lock */
+	u64 ncf_srv_cinfo;
 	u64 ncf_cur_fsize;
 };
 
@@ -1041,7 +1043,7 @@ static inline bool try_to_expire_client(struct nfs4_client *clp)
 }
 
 extern __be32 nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp,
-		struct dentry *dentry, struct nfs4_delegation **pdp);
+		const struct path *path, struct nfs4_delegation **pdp);
 
 struct nfsd4_get_dir_delegation;
 struct nfs4_delegation *nfsd_get_dir_deleg(struct nfsd4_compound_state *cstate,
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 02df2ff02d60..260ba57b5542 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -631,6 +631,10 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		}
 	}
 
+	if (attr->na_deleg)
+		nfsd4_vet_deleg_attrs(iap, attr->na_deleg,
+				      attr->na_deleg_attrs);
+
 	for (retries = 1;;) {
 		struct iattr attrs;
 
@@ -646,6 +650,8 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		if (!nfsd_wait_for_delegreturn(rqstp, inode))
 			break;
 	}
+	if (attr->na_deleg && !host_err)
+		nfsd4_record_deleg_attrs(iap, attr->na_deleg);
 	if (attr->na_seclabel && attr->na_seclabel->len)
 		attr->na_labelerr = security_inode_setsecctx(dentry,
 			attr->na_seclabel->data, attr->na_seclabel->len);
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 73ab9d34a275..78cfa0dcaf5c 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -103,6 +103,10 @@ struct nfsd_attrs {
 	struct posix_acl	*na_pacl;	/* input */
 	struct posix_acl	*na_dpacl;	/* input */
 
+	/* input: vet and record these FATTR4_WORD2_TIME_DELEG_* times */
+	struct nfs4_delegation	*na_deleg;
+	u32			na_deleg_attrs;
+
 	int			na_labelerr;	/* output */
 	int			na_dpaclerr;	/* output */
 	int			na_paclerr;	/* output */

-- 
2.56.0

[PATCH v3 3/8] nfsd: don't stamp over an applied delegated mtime at DELEGRETURN

[Jeff Layton]

nfsd4_finalize_deleg_timestamps() stamps a/m/ctime to "now" at
DELEGRETURN when the file was written and dl_setattr is clear. But
dl_setattr is only set when both the mtime and ctime vets pass:

- nfsd4_vet_deleg_attrs() applies the holder's mtime but leaves
  dl_setattr clear if that mtime doesn't pass dl_ctime. A client mtime a
  fraction of a millisecond behind the server's ctime at grant is
  enough.
- cb_getattr_update_times() applies the holder's mtime but never sets
  dl_setattr.

Either way, DELEGRETURN then overwrites the mtime the holder supplied
with the return time. Seen as git t1092 failures over NFSv4.2: an
untouched file's c/mtime jump ~25s after git recorded them.

- Set dl_setattr whenever the holder's mtime is applied. Test it under
  inode_lock at DELEGRETURN, so a racing CB_GETATTR reply's mtime isn't
  stamped over.
- If that mtime doesn't pass dl_ctime, set ctime to the current time
  instead of leaving it at its grant value. DELEGRETURN no longer
  stamps it, and where the change attribute derives from ctime (no
  i_version), it must still move across the holder's writes. dl_ctime
  is recorded from the inode, so later vetting sees that ctime. RFC 9754
  specifies the change time only when the mtime exceeds it; RFC 8881
  section 10.4.3 says the server SHOULD keep time_metadata and change
  from going backward.
- Have SETATTR and CB_GETATTR share nfsd4_vet_deleg_attrs(), so their
  vetting can't diverge again.

The client fix "NFS: don't let delegated c/mtime stamps go backward"
makes the ctime case rare.

Fixes: e5e9b24ab8fa ("nfsd: freeze c/mtime updates with outstanding WRITE_ATTRS delegation")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 82 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 47 insertions(+), 35 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a8966d1f786f..35712e129b7a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1463,6 +1463,12 @@ static void put_deleg_file(struct nfs4_file *fp)
 
 static void nfsd4_finalize_deleg_timestamps(struct nfs4_delegation *dp, struct file *f)
 {
+	struct inode *inode = file_inode(f);
+	struct iattr attr = {
+		.ia_valid = ATTR_ATIME | ATTR_CTIME | ATTR_MTIME | ATTR_DELEG,
+	};
+	int ret;
+
 	/* don't do anything if FMODE_NOCMTIME isn't set */
 	if ((READ_ONCE(f->f_mode) & FMODE_NOCMTIME) == 0)
 		return;
@@ -1475,12 +1481,22 @@ static void nfsd4_finalize_deleg_timestamps(struct nfs4_delegation *dp, struct f
 	if (!READ_ONCE(dp->dl_written))
 		return;
 
+	/* A racing CB_GETATTR reply sets dl_setattr under inode_lock */
+	inode_lock(inode);
 	/* did it get a setattr for the timestamps at some point? */
-	if (dp->dl_setattr)
+	if (dp->dl_setattr) {
+		inode_unlock(inode);
 		return;
+	}
 
 	/* Stamp everything to "now" */
-	nfsd_update_cmtime_attr(f, ATTR_ATIME);
+	ret = notify_change(&nop_mnt_idmap, f->f_path.dentry, &attr, NULL);
+	inode_unlock(inode);
+	if (ret)
+		pr_notice_ratelimited("nfsd: Unable to update timestamps on inode %02x:%02x:%llu: %d\n",
+				      MAJOR(inode->i_sb->s_dev),
+				      MINOR(inode->i_sb->s_dev),
+				      inode->i_ino, ret);
 }
 
 static void nfs4_unlock_deleg_lease(struct nfs4_delegation *dp)
@@ -10720,14 +10736,16 @@ bool nfsd4_vet_deleg_time(struct timespec64 *req, const struct timespec64 *orig,
 }
 
 /**
- * nfsd4_vet_deleg_attrs - vet the delegated timestamps in a SETATTR
- * @iattr: attributes to be set
- * @dp: timestamp delegation held by the SETATTR's sender
- * @deleg_attrs: FATTR4_WORD2_TIME_DELEG_* bits in the SETATTR
+ * nfsd4_vet_deleg_attrs - vet a timestamp delegation holder's times
+ * @iattr: holder's times in ia_atime and ia_mtime
+ * @dp: holder's timestamp delegation
+ * @deleg_attrs: FATTR4_WORD2_TIME_DELEG_* bits naming the times to vet
  *
- * Drop requested timestamps that don't pass nfsd4_vet_deleg_time(), and
- * clamp ones in the future. Set dl_setattr if both the mtime and ctime
- * pass.
+ * Set the ATTR_[AMC]TIME* bits in @iattr for the times that pass
+ * nfsd4_vet_deleg_time(), clamping ones in the future, and clear them for
+ * the ones that don't. Ctime follows an applied mtime, or is set to the
+ * current time if that mtime doesn't pass dl_ctime: DELEGRETURN won't
+ * stamp it, and the change attribute may derive from it.
  *
  * Context: Caller holds the inode_lock across this, applying @iattr, and
  * nfsd4_record_deleg_attrs(), so cb_getattr_update_times() can't move
@@ -10741,21 +10759,21 @@ void nfsd4_vet_deleg_attrs(struct iattr *iattr, struct nfs4_delegation *dp,
 
 	lockdep_assert_held_write(&inode->i_rwsem);
 
-	if ((deleg_attrs & FATTR4_WORD2_TIME_DELEG_ACCESS) &&
-	    !nfsd4_vet_deleg_time(&iattr->ia_atime, &dp->dl_atime, &now))
+	if (deleg_attrs & FATTR4_WORD2_TIME_DELEG_ACCESS) {
 		iattr->ia_valid &= ~(ATTR_ATIME | ATTR_ATIME_SET);
+		if (nfsd4_vet_deleg_time(&iattr->ia_atime, &dp->dl_atime, &now))
+			iattr->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET;
+	}
 
 	if (deleg_attrs & FATTR4_WORD2_TIME_DELEG_MODIFY) {
-		if (nfsd4_vet_deleg_time(&iattr->ia_mtime, &dp->dl_mtime, &now)) {
-			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);
-		} else {
-			iattr->ia_valid &= ~(ATTR_CTIME | ATTR_CTIME_SET |
-					     ATTR_MTIME | ATTR_MTIME_SET);
-		}
+		iattr->ia_valid &= ~(ATTR_MTIME | ATTR_MTIME_SET |
+				     ATTR_CTIME | ATTR_CTIME_SET);
+		if (!nfsd4_vet_deleg_time(&iattr->ia_mtime, &dp->dl_mtime, &now))
+			return;
+		iattr->ia_valid |= ATTR_MTIME | ATTR_MTIME_SET | ATTR_CTIME;
+		iattr->ia_ctime = iattr->ia_mtime;
+		if (nfsd4_vet_deleg_time(&iattr->ia_ctime, &dp->dl_ctime, &now))
+			iattr->ia_valid |= ATTR_CTIME_SET;
 	}
 }
 
@@ -10778,9 +10796,12 @@ void nfsd4_record_deleg_attrs(const struct iattr *iattr,
 
 	if (iattr->ia_valid & ATTR_ATIME_SET)
 		dp->dl_atime = iattr->ia_atime;
-	if (iattr->ia_valid & ATTR_MTIME_SET)
+	if (iattr->ia_valid & ATTR_MTIME_SET) {
 		dp->dl_mtime = iattr->ia_mtime;
-	/* inode_set_ctime_deleg() keeps a later ctime */
+		/* Don't let DELEGRETURN stamp over the holder's mtime */
+		dp->dl_setattr = true;
+	}
+	/* Ctime may be "now", or inode_set_ctime_deleg() kept a later one */
 	if (iattr->ia_valid & ATTR_CTIME)
 		dp->dl_ctime = inode_get_ctime(inode);
 }
@@ -10810,8 +10831,6 @@ static void cb_getattr_update_times(const struct path *path,
 	/* Serialize vetting against dl_*time with their update below */
 	inode_lock(inode);
 	if (deleg_attrs_deleg(dp->dl_type)) {
-		struct timespec64 now = current_time(inode);
-
 		/*
 		 * The holder's own SETATTRs, xattr changes, fallocates, LINKs
 		 * etc. don't break its delegation and don't show up in its
@@ -10826,16 +10845,9 @@ static void cb_getattr_update_times(const struct path *path,
 
 		attrs.ia_atime = ncf->ncf_cb_atime;
 		attrs.ia_mtime = ncf->ncf_cb_mtime;
-
-		if (nfsd4_vet_deleg_time(&attrs.ia_atime, &dp->dl_atime, &now))
-			attrs.ia_valid |= ATTR_ATIME | ATTR_ATIME_SET;
-
-		if (nfsd4_vet_deleg_time(&attrs.ia_mtime, &dp->dl_mtime, &now)) {
-			attrs.ia_valid |= ATTR_MTIME | ATTR_MTIME_SET;
-			attrs.ia_ctime = attrs.ia_mtime;
-			if (nfsd4_vet_deleg_time(&attrs.ia_ctime, &dp->dl_ctime, &now))
-				attrs.ia_valid |= ATTR_CTIME | ATTR_CTIME_SET;
-		}
+		nfsd4_vet_deleg_attrs(&attrs, dp,
+				      FATTR4_WORD2_TIME_DELEG_ACCESS |
+				      FATTR4_WORD2_TIME_DELEG_MODIFY);
 
 		/*
 		 * The times are the holder's, so don't hold the GETATTR

-- 
2.56.0

[PATCH v3 4/8] nfsd: don't vet delegated times against an exclusive-create verifier

[Jeff Layton]

An exclusive create stores the client's verifier in atime and mtime
until the client sets real times. A delegation granted on that OPEN
records those as dl_atime and dl_mtime (e.g. 4777.0 and 681509.0), so
vetting the holder's times against them always passes, including times
older than the file itself.

When the OPEN did an exclusive create, use the ctime at grant, less 1ns,
as the floor for atime and mtime too. Vetting rejects a time equal to
its floor, and a holder time equal to the ctime must still replace the
verifier. Do this for both write and read timestamp delegations, and
set a read delegation's floors only when the stat behind them was
done. Move nfsd4_create_is_exclusive() to xdr4.h so nfs4state.c can use
it.

Fixes: 7663e963a511 ("nfsd: track original timestamps in nfs4_delegation")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4proc.c  |  6 ------
 fs/nfsd/nfs4state.c | 27 +++++++++++++++++++++++----
 fs/nfsd/xdr4.h      |  6 ++++++
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 4e5d5852bf57..25613c1ffaaf 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -245,12 +245,6 @@ static void nfsd4_set_open_owner_reply_cache(struct nfsd4_compound_state *cstate
 			&resfh->fh_handle);
 }
 
-static inline bool nfsd4_create_is_exclusive(int createmode)
-{
-	return createmode == NFS4_CREATE_EXCLUSIVE ||
-		createmode == NFS4_CREATE_EXCLUSIVE4_1;
-}
-
 static struct file *do_lookup_open(struct path *parent,
 				   struct qstr *name,
 				   unsigned int oflags,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 35712e129b7a..4566fa86f82f 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7466,6 +7466,26 @@ nfsd4_add_rdaccess_to_wrdeleg(struct svc_rqst *rqstp, struct nfsd4_open *open,
 	return true;
 }
 
+/*
+ * An exclusive create stores the verifier in atime and mtime, so the only
+ * real floor for the holder's times is the ctime. Vetting rejects a time
+ * equal to its floor, but one equal to the ctime must still replace the
+ * verifier.
+ */
+static void nfsd4_deleg_set_time_floors(struct nfs4_delegation *dp,
+					const struct nfsd4_open *open,
+					const struct kstat *stat)
+{
+	dp->dl_atime = stat->atime;
+	dp->dl_ctime = stat->ctime;
+	dp->dl_mtime = stat->mtime;
+	if (open->op_created &&
+	    nfsd4_create_is_exclusive(open->op_createmode)) {
+		dp->dl_atime = timespec64_sub(stat->ctime, ns_to_timespec64(1));
+		dp->dl_mtime = dp->dl_atime;
+	}
+}
+
 /*
  * The Linux NFS server does not offer write delegations to NFSv4.0
  * clients in order to avoid conflicts between write delegations and
@@ -7556,9 +7576,7 @@ nfs4_open_delegation(struct svc_rqst *rqstp, struct nfsd4_open *open,
 						    OPEN_DELEGATE_WRITE;
 		dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
 		dp->dl_cb_fattr.ncf_srv_cinfo = dp->dl_cb_fattr.ncf_initial_cinfo;
-		dp->dl_atime = stat.atime;
-		dp->dl_ctime = stat.ctime;
-		dp->dl_mtime = stat.mtime;
+		nfsd4_deleg_set_time_floors(dp, open, &stat);
 		spin_lock(&f->f_lock);
 		if (deleg_ts)
 			f->f_mode |= FMODE_NOCMTIME;
@@ -7567,7 +7585,8 @@ nfs4_open_delegation(struct svc_rqst *rqstp, struct nfsd4_open *open,
 	} else {
 		open->op_delegate_type = deleg_ts && nfs4_delegation_stat(dp, currentfh, &stat) ?
 					 OPEN_DELEGATE_READ_ATTRS_DELEG : OPEN_DELEGATE_READ;
-		dp->dl_atime = stat.atime;
+		if (open->op_delegate_type == OPEN_DELEGATE_READ_ATTRS_DELEG)
+			nfsd4_deleg_set_time_floors(dp, open, &stat);
 		trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
 	}
 	nfs4_put_stid(&dp->dl_stid);
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index ecab2249e9bd..feda5464f260 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -280,6 +280,12 @@ struct nfsd4_open {
 	struct svc_rqst *op_rqstp;
 };
 
+static inline bool nfsd4_create_is_exclusive(int createmode)
+{
+	return createmode == NFS4_CREATE_EXCLUSIVE ||
+		createmode == NFS4_CREATE_EXCLUSIVE4_1;
+}
+
 struct nfsd4_open_confirm {
 	stateid_t	oc_req_stateid		/* request */;
 	u32		oc_seqid    		/* request */;

-- 
2.56.0

[PATCH v3 5/8] nfsd: mark the delegation written on COPY/CLONE instead of stamping c/mtime

[Jeff Layton]

Under a timestamp delegation, nfsd stamps the destination's c/mtime to
the server's "now" after a COPY or CLONE. Since commit 6e7daa3dad29
("NFSv4.2: fix CLONE/COPY attrs in presence of delegated attributes"),
the client also stamps its own c/mtime afterwards and ignores the
server's. If the client's coarse stamp lands even slightly before the
server's, the server can't move ctime back when the holder returns its
times, and ctime changes after DELEGRETURN. COPY returns no attributes,
so the client can't avoid this.

Treat COPY/CLONE like WRITE: mark the delegation written and let the
holder supply the times. If the holder never returns them, DELEGRETURN
stamps them via nfsd4_finalize_deleg_timestamps(); other clients'
GETATTRs go through CB_GETATTR. The async COPY thread has no
filehandle, so look up the nfs4_file at launch, and have the thread
mark the delegation when the copy has written something. Do the lookup
for every async COPY, so a delegation granted while the copy ran is
covered too. nfsd_update_cmtime_attr() is left without callers; remove
it.

Fixes: 2863bac7f49c ("nfsd: update mtime/ctime on CLONE in presense of delegated attributes")
Fixes: 4183cf383b6f ("nfsd: update mtime/ctime on COPY in presence of delegated attributes")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4proc.c  | 48 +++++++++++++++++++++++++++++++++++++-----------
 fs/nfsd/nfs4state.c | 30 +-----------------------------
 fs/nfsd/state.h     |  2 +-
 fs/nfsd/xdr4.h      |  3 ++-
 4 files changed, 41 insertions(+), 42 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 25613c1ffaaf..a65e13f0f77a 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1438,6 +1438,25 @@ static void nfsd4_file_mark_deleg_written(struct nfs4_file *fi)
 	spin_unlock(&fi->fi_lock);
 }
 
+/*
+ * A COPY or CLONE into a file under a timestamp delegation is the holder's
+ * modification, like a WRITE: the holder supplies the c/mtime, and DELEGRETURN
+ * stamps them if it doesn't.
+ */
+static void nfsd4_copy_mark_deleg_written(struct svc_fh *fhp,
+					  struct nfsd_file *dst)
+{
+	struct nfs4_file *fi;
+
+	if (!(READ_ONCE(dst->nf_file->f_mode) & FMODE_NOCMTIME))
+		return;
+	fi = nfsd4_file_hash_lookup(fhp);
+	if (fi) {
+		nfsd4_file_mark_deleg_written(fi);
+		put_nfs4_file(fi);
+	}
+}
+
 static __be32
 nfsd4_write(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	    union nfsd4_op_u *u)
@@ -1576,8 +1595,8 @@ nfsd4_clone(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	}
 	status = nfserrno(host_err);
 
-	if (!status && (READ_ONCE(dst->nf_file->f_mode) & FMODE_NOCMTIME) != 0)
-		nfsd_update_cmtime_attr(dst->nf_file, 0);
+	if (!status)
+		nfsd4_copy_mark_deleg_written(&cstate->current_fh, dst);
 
 	nfsd_file_put(dst);
 	nfsd_file_put(src);
@@ -2198,6 +2217,10 @@ static void release_copy_files(struct nfsd4_copy *copy)
 		nfsd_file_put(copy->nf_dst);
 		copy->nf_dst = NULL;
 	}
+	if (copy->cp_dst_fi) {
+		put_nfs4_file(copy->cp_dst_fi);
+		copy->cp_dst_fi = NULL;
+	}
 }
 
 /*
@@ -2301,8 +2324,9 @@ static int nfsd4_do_async_copy(void *data)
 	set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags);
 	trace_nfsd_copy_async_done(async->cp_stid.sc_client, copy);
 	atomic_dec(&copy->cp_nn->pending_async_copies);
-	if (copy->cp_res.wr_bytes_written > 0 && copy->attr_update)
-		nfsd_update_cmtime_attr(copy->nf_dst->nf_file, 0);
+	/* The holder may DELEGRETURN as soon as it sees CB_OFFLOAD */
+	if (copy->cp_res.wr_bytes_written > 0 && copy->cp_dst_fi)
+		nfsd4_file_mark_deleg_written(copy->cp_dst_fi);
 	nfsd4_send_cb_offload(async);
 	/* Drop the kthread's reference (taken in nfsd4_copy()); copy may be freed after this. */
 	nfs4_put_copy(async);
@@ -2382,9 +2406,12 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		 * the callback stateid zeroed.
 		 */
 		dup_copy_fields(copy, &async_copy->cp_copy);
-		if ((READ_ONCE(copy->nf_dst->nf_file->f_mode) &
-			       FMODE_NOCMTIME) != 0)
-			async_copy->cp_copy.attr_update = true;
+		/*
+		 * The copy thread has no filehandle. A timestamp delegation
+		 * may be granted while the copy runs.
+		 */
+		async_copy->cp_copy.cp_dst_fi =
+			nfsd4_file_hash_lookup(&cstate->current_fh);
 		memcpy(async_copy->cp_cb_offload.co_referring_sessionid.data,
 		       cstate->session->se_sessionid.data,
 		       NFS4_MAX_SESSIONID_LEN);
@@ -2415,10 +2442,9 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	} else {
 		status = nfsd4_do_copy(copy, copy->nf_src->nf_file,
 				       copy->nf_dst->nf_file, true);
-		if ((READ_ONCE(copy->nf_dst->nf_file->f_mode) &
-			       FMODE_NOCMTIME) != 0 &&
-				copy->cp_res.wr_bytes_written > 0)
-			nfsd_update_cmtime_attr(copy->nf_dst->nf_file, 0);
+		if (copy->cp_res.wr_bytes_written > 0)
+			nfsd4_copy_mark_deleg_written(&cstate->current_fh,
+						      copy->nf_dst);
 	}
 out:
 	trace_nfsd_copy_done(copy, status);
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 4566fa86f82f..3bb518270b8b 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6304,7 +6304,7 @@ move_to_close_lru(struct nfs4_ol_stateid *s, struct net *net)
 		nfs4_put_stid(&last->st_stid);
 }
 
-static noinline_for_stack struct nfs4_file *
+noinline_for_stack struct nfs4_file *
 nfsd4_file_hash_lookup(const struct svc_fh *fhp)
 {
 	struct inode *inode = d_inode(fhp->fh_dentry);
@@ -11177,34 +11177,6 @@ nfsd_get_dir_deleg(struct nfsd4_compound_state *cstate,
 	return ERR_PTR(status);
 }
 
-/**
- * nfsd_update_cmtime_attr - update file's delegated ctime/mtime,
- *                           and optionally other attributes (ie ATTR_ATIME).
- * @f: pointer to an opened file
- * @flags: any additional flags that should be updated
- *
- * Given upon opening a file delegated attributes were issues, update
- * @f attributes to current times.
- */
-void nfsd_update_cmtime_attr(struct file *f, unsigned int flags)
-{
-	int ret;
-	struct inode *inode = file_inode(f);
-	struct iattr attr = {
-		.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_DELEG | flags,
-	};
-
-	inode_lock(inode);
-	ret = notify_change(&nop_mnt_idmap, f->f_path.dentry, &attr, NULL);
-	inode_unlock(inode);
-	if (ret)
-		pr_notice_ratelimited("nfsd: Unable to update timestamps on "
-				      "inode %02x:%02x:%llu: %d\n",
-				      MAJOR(inode->i_sb->s_dev),
-				      MINOR(inode->i_sb->s_dev),
-				      inode->i_ino, ret);
-}
-
 static void
 nfsd4_run_cb_notify(struct nfsd4_cb_notify *ncn)
 {
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 9e8699810e33..8a189f1b0133 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -981,13 +981,13 @@ void nfsd4_get_client(struct nfs4_client *clp);
 void nfsd4_put_client(struct nfs4_client *clp);
 void nfsd4_async_copy_reaper(struct nfsd_net *nn);
 bool nfsd4_has_active_async_copies(struct nfs4_client *clp);
-void nfsd_update_cmtime_attr(struct file *f, unsigned int flags);
 extern struct nfs4_client_reclaim *nfs4_client_to_reclaim(struct xdr_netobj name,
 				struct xdr_netobj princhash, struct nfsd_net *nn);
 int nfsd_handle_dir_event(u32 mask, const struct inode *dir, const void *data,
 			  int data_type, const struct qstr *name);
 
 void put_nfs4_file(struct nfs4_file *fi);
+struct nfs4_file *nfsd4_file_hash_lookup(const struct svc_fh *fhp);
 extern void nfs4_put_cpntf_state(struct nfsd_net *nn,
 				 struct nfs4_cpntf_state *cps);
 extern __be32 manage_cpntf_state(struct nfsd_net *nn, stateid_t *st,
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index feda5464f260..8cebeafbfc20 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -646,7 +646,8 @@ struct nfsd4_copy {
 
 	struct nfsd_file        *nf_src;
 	struct nfsd_file        *nf_dst;
-	bool			attr_update;
+	/* async: file whose timestamp delegation the copy marks written */
+	struct nfs4_file	*cp_dst_fi;
 
 	struct nfsd4_ssc_umount_item *ss_nsui;
 	struct nfs_fh		c_fh;

-- 
2.56.0

[PATCH v3 6/8] fs: check a fine-grained time before clamping a delegated ctime

[Jeff Layton]

inode_set_ctime_deleg() clamps an update later than the coarse time
(or mg floor) to that time. An NFS client holding a timestamp delegation
stamps at least 1ns after the fine-grained ctime the server last
reported. If the delegation comes back within the same coarse tick,
that stamp looks like it's in the future: ctime is clamped back, and
the ctime the client reported while holding the delegation changes
after DELEGRETURN.

When the update is past the coarse time, take a fine-grained time with
ktime_get_real_ts64_mg() and clamp only if the update is past that too.
That also raises the floor to at least the update, so later coarse
stamps can't land before it. This works for non-multigrain filesystems
too: their stamps also read the floor.

Fixes: 7f2c86cba3c5 ("fs: handle delegated timestamps in setattr_copy_mgtime")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/inode.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index f63535ee3629..dc744fa39b75 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2991,9 +2991,19 @@ struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 u
 
 	ktime_get_coarse_real_ts64_mg(&now);
 
-	/* Clamp the update to "now" if it's in the future */
-	if (timespec64_compare(&update, &now) > 0)
-		update = now;
+	/*
+	 * Clamp the update to "now" if it's in the future. The coarse time
+	 * can trail an update just past a fine-grained ctime handed out in
+	 * the current tick, so check a fine-grained time before clamping.
+	 * That also raises the floor past the update, so later coarse
+	 * stamps can't land before it.
+	 */
+	if (timespec64_compare(&update, &now) > 0) {
+		ktime_get_real_ts64_mg(&now);
+		mgtime_counter_inc(mg_fine_stamps);
+		if (timespec64_compare(&update, &now) > 0)
+			update = now;
+	}
 
 	update = timestamp_truncate(update, inode);
 

-- 
2.56.0

[PATCH v3 7/8] nfsd: check a fine-grained time before clamping a delegated mtime

[Jeff Layton]

nfsd4_vet_deleg_attrs() clamps a holder's mtime to current_time() if
the mtime is later. current_time() is coarse unless the filesystem is
multigrain and the inode's ctime was queried and is still equal to the
coarse time. A client holding a timestamp delegation stamps at least
1ns after the fine-grained ctime it was last sent, so an mtime returned
within the same coarse tick is clamped back, and the mtime the client
reported changes after DELEGRETURN. On non-multigrain exports that is
the normal case.

When the mtime is past the coarse time, clamp it and its ctime against
ktime_get_real_ts64() instead. Such an mtime also passes dl_ctime, so
its ctime goes through inode_set_ctime_deleg(), which raises the mgtime
floor past it; later stamps on other files can't appear older. atime
gets no such floor raise, so leave it clamped to the coarse time.

This depends on "fs: check a fine-grained time before clamping a
delegated ctime". Without it, ctime is still clamped to the coarse time
and the floor isn't raised, leaving mtime ahead of both.

Fixes: 6ae30d6eb26b ("nfsd: add support for delegated timestamps")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 3bb518270b8b..03e694f635db 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -10785,6 +10785,17 @@ void nfsd4_vet_deleg_attrs(struct iattr *iattr, struct nfs4_delegation *dp,
 	}
 
 	if (deleg_attrs & FATTR4_WORD2_TIME_DELEG_MODIFY) {
+		/*
+		 * A holder stamps just past the last fine-grained ctime it
+		 * was sent, which can be past the coarse @now. Clamp against
+		 * a fine-grained time instead. An mtime past @now also passes
+		 * dl_ctime, so its ctime goes through inode_set_ctime_deleg(),
+		 * which raises the mgtime floor past it. atime gets no such
+		 * floor raise, so it stays clamped to the coarse time.
+		 */
+		if (timespec64_compare(&iattr->ia_mtime, &now) > 0)
+			ktime_get_real_ts64(&now);
+
 		iattr->ia_valid &= ~(ATTR_MTIME | ATTR_MTIME_SET |
 				     ATTR_CTIME | ATTR_CTIME_SET);
 		if (!nfsd4_vet_deleg_time(&iattr->ia_mtime, &dp->dl_mtime, &now))

-- 
2.56.0

[PATCH v3 8/8] nfsd: raise delegated time floors after a size-changing SETATTR

[Jeff Layton]

A size-changing SETATTR stamps c/mtime on the server
(__nfsd_setattr(), RFC 8881 section 18.30.4), but dl_mtime and dl_ctime
stay at their earlier values. A holder mtime from before the truncate
still passes vetting and moves mtime back past the truncate. The Linux
client can send one: if a delegation return starts while
nfs_vmtruncate() has i_lock dropped, DELEGRETURN carries the
pre-truncate mtime.

When the SETATTR uses a timestamp delegation's stateid, have
nfsd_setattr() record the c/mtime the size change set in dl_mtime and
dl_ctime. A holder time at or before the server's own stamp is stale.

Fixes: 6ae30d6eb26b ("nfsd: add support for delegated timestamps")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4proc.c  |  4 ++++
 fs/nfsd/nfs4state.c | 10 +++++++---
 fs/nfsd/vfs.h       |  2 +-
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index a65e13f0f77a..7fef904c3366 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1379,6 +1379,10 @@ nfsd4_setattr(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 				status = nfs_ok;
 			}
 		}
+	} else if (st && (st->sc_type & SC_TYPE_DELEG) &&
+		   deleg_attrs_deleg(delegstateid(st)->dl_type)) {
+		/* Have nfsd_setattr() record the c/mtime the size change sets */
+		attrs.na_deleg = delegstateid(st);
 	}
 	if (status)
 		goto out_err;
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 03e694f635db..606c4b469728 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -10812,8 +10812,9 @@ void nfsd4_vet_deleg_attrs(struct iattr *iattr, struct nfs4_delegation *dp,
  * @iattr: vetted attributes, not the copy notify_change() altered
  * @dp: timestamp delegation whose holder supplied the times
  *
- * Set dl_*time to the times applied, so a stale holder time can't move
- * them backward and a repeated CB_GETATTR reply isn't reapplied.
+ * Set dl_*time to the times applied, or to the c/mtime a size change
+ * stamped, so a stale holder time can't move them backward and a repeated
+ * CB_GETATTR reply isn't reapplied.
  *
  * Context: Caller holds the inode_lock.
  */
@@ -10830,9 +10831,12 @@ void nfsd4_record_deleg_attrs(const struct iattr *iattr,
 		dp->dl_mtime = iattr->ia_mtime;
 		/* Don't let DELEGRETURN stamp over the holder's mtime */
 		dp->dl_setattr = true;
+	} else if (iattr->ia_valid & ATTR_SIZE) {
+		/* RFC 8881 section 18.30.4: a size change sets mtime */
+		dp->dl_mtime = inode_get_mtime(inode);
 	}
 	/* Ctime may be "now", or inode_set_ctime_deleg() kept a later one */
-	if (iattr->ia_valid & ATTR_CTIME)
+	if (iattr->ia_valid & (ATTR_CTIME | ATTR_SIZE))
 		dp->dl_ctime = inode_get_ctime(inode);
 }
 
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 78cfa0dcaf5c..34ec8293e7b8 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -103,7 +103,7 @@ struct nfsd_attrs {
 	struct posix_acl	*na_pacl;	/* input */
 	struct posix_acl	*na_dpacl;	/* input */
 
-	/* input: vet and record these FATTR4_WORD2_TIME_DELEG_* times */
+	/* input: timestamp delegation to vet and record times against */
 	struct nfs4_delegation	*na_deleg;
 	u32			na_deleg_attrs;
 

-- 
2.56.0