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

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

[Jeff Layton]

The main problem is that an attribute delegation has strict rules about
updates can be done, and that prevents applications that backdate
timestamps on files after writing them, (e.g. tar -x, cp -p, etc.) from
setting them properly.

We could fix this on the client by making it always issue an RPC for the
SETATTR, but that would harm performance in these common use-cases.
This set allows the server to accept some of these backdated mtime
updates, by changing how the ctime is handled.

It'd be nice to see this in v7.4.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v2:
- Split the change into three patches
- Stamp now in inode_set_ctime_deleg() for a stale update, so nfsd need
  not compare
- Drop a SETATTR report that matches the inode, so an untouched file
  keeps its change attribute
- Set dl_setattr only on the branch that carries a c/mtime update
- Compare against the inode, not the grant-time snapshots (Chuck)
- Set the ctime whenever a CB_GETATTR moves the mtime
- Take i_rwsem before the CB_GETATTR comparison
- Drop the unused dl_atime, dl_mtime and dl_ctime, and make
  nfsd4_vet_deleg_time() static
- Link to v1: https://lore.kernel.org/r/20260901-delegts-v1-1-9937f7ee4370@kernel.org

---
Jeff Layton (3):
      fs: stamp the current time for a stale delegated ctime update
      nfsd: accept a backdated timestamp from a delegation holder
      nfsd: compare CB_GETATTR times against the inode

 fs/inode.c          | 17 +++++++++++----
 fs/nfsd/nfs4proc.c  | 63 +++++++++++++++++++++++++++++++++++++++++++----------
 fs/nfsd/nfs4state.c | 49 ++++++++++++++++++++++++++---------------
 fs/nfsd/state.h     |  8 -------
 4 files changed, 95 insertions(+), 42 deletions(-)
---
base-commit: ac579868af0c900d8fd9c8cfe9e10db46ccc5a75
change-id: 20260901-delegts-e91a648f1427

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

[PATCH v2 1/3] fs: stamp the current time for a stale delegated ctime update

[Jeff Layton]

inode_set_ctime_deleg() drops an update that does not advance the ctime.
That is correct when a client reports a timestamp that it advanced on its
own. It is wrong when the client moves the timestamp backwards on purpose,
as utimensat() does.

Stamp the current time in that case. The ctime still never moves backwards.

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

diff --git a/fs/inode.c b/fs/inode.c
index ba7da39be4a3..8391814a4016 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2959,11 +2959,17 @@ EXPORT_SYMBOL(inode_set_ctime_current);
  * inode attributes, including the mtime. When updating the mtime, update
  * the ctime to a value at least equal to that.
  *
- * This can race with concurrent updates to the inode, in which
- * case the update is skipped.
+ * The ctime never moves backwards. An @update that does not advance the ctime
+ * records the current time instead, so that the delegated change is still
+ * visible in the ctime.
+ *
+ * This can still race with a concurrent update to the inode. That stamp takes
+ * precedence, and is at least as recent as the one it displaces.
  *
  * Note that this works even when multigrain timestamps are not enabled,
  * so it is used in either case.
+ *
+ * Returns the resulting ctime.
  */
 struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 update)
 {
@@ -2975,9 +2981,12 @@ struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 u
 	cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED;
 	cur_ts.tv_sec = inode_get_ctime_sec(inode);
 
-	/* If the update is older than the existing value, skip it. */
+	/*
+	 * The update does not advance the ctime. Stamp the current time, so
+	 * that the delegated change is still visible in the ctime.
+	 */
 	if (timespec64_compare(&update, &cur_ts) <= 0)
-		return cur_ts;
+		return inode_set_ctime_current(inode);
 
 	ktime_get_coarse_real_ts64_mg(&now);
 

-- 
2.55.0

Re: [PATCH v2 1/3] fs: stamp the current time for a stale delegated ctime update

[Jan Kara]

On Wed 02-09-26 14:54:14, Jeff Layton wrote:
> inode_set_ctime_deleg() drops an update that does not advance the ctime.
> That is correct when a client reports a timestamp that it advanced on its
> own. It is wrong when the client moves the timestamp backwards on purpose,
> as utimensat() does.
> 
> Stamp the current time in that case. The ctime still never moves backwards.
> 
> Fixes: 3952f1cbcbc4 ("nfsd: fix SETATTR updates for delegated timestamps")
> Assisted-by: LLM
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/inode.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index ba7da39be4a3..8391814a4016 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2959,11 +2959,17 @@ EXPORT_SYMBOL(inode_set_ctime_current);
>   * inode attributes, including the mtime. When updating the mtime, update
>   * the ctime to a value at least equal to that.
>   *
> - * This can race with concurrent updates to the inode, in which
> - * case the update is skipped.
> + * The ctime never moves backwards. An @update that does not advance the ctime
> + * records the current time instead, so that the delegated change is still
> + * visible in the ctime.
> + *
> + * This can still race with a concurrent update to the inode. That stamp takes
> + * precedence, and is at least as recent as the one it displaces.
>   *
>   * Note that this works even when multigrain timestamps are not enabled,
>   * so it is used in either case.
> + *
> + * Returns the resulting ctime.
>   */
>  struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 update)
>  {
> @@ -2975,9 +2981,12 @@ struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 u
>  	cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED;
>  	cur_ts.tv_sec = inode_get_ctime_sec(inode);
>  
> -	/* If the update is older than the existing value, skip it. */
> +	/*
> +	 * The update does not advance the ctime. Stamp the current time, so
> +	 * that the delegated change is still visible in the ctime.
> +	 */
>  	if (timespec64_compare(&update, &cur_ts) <= 0)
> -		return cur_ts;
> +		return inode_set_ctime_current(inode);
>  
>  	ktime_get_coarse_real_ts64_mg(&now);
>  
> 
> -- 
> 2.55.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

[Jeff Layton]

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

[PATCH v2 3/3] nfsd: compare CB_GETATTR times against the inode

[Jeff Layton]

cb_getattr_update_times() vets the times that a client reports in a
CB_GETATTR against dl_atime, dl_mtime and dl_ctime. nfsd samples those
three when it grants the delegation, and nothing refreshes them. An
earlier CB_GETATTR or SETATTR can move the inode past them.

The vetting then compares against a stale value and accepts a report
that it must reject. setattr_copy() applies the atime and the mtime with
no check of the current value, so both move backwards. That is the
result the vetting exists to prevent.

Compare against the inode instead. Take the inode lock before the
comparison, so that a setattr from a concurrent delegation break cannot
land before notify_change() runs.

With this change, there is no longer a need to keep dl_atime/mtime/ctime
in the delegation. Drop those fields as well. That also drops a read of
an uninitialized struct kstat: the read-delegation arm assigned
stat.atime to dl_atime even when nfs4_delegation_stat() had not run.
Nothing consumed the value, since dl_atime is only read under
deleg_attrs_deleg().

Fixes: 3952f1cbcbc4 ("nfsd: fix SETATTR updates for delegated timestamps")
Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 49 +++++++++++++++++++++++++++++++------------------
 fs/nfsd/state.h     |  8 --------
 2 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ae0af9490fb1..b2b8d3a8030b 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7307,9 +7307,6 @@ 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_atime = stat.atime;
-		dp->dl_ctime = stat.ctime;
-		dp->dl_mtime = stat.mtime;
 		spin_lock(&f->f_lock);
 		if (deleg_ts)
 			f->f_mode |= FMODE_NOCMTIME;
@@ -7318,7 +7315,6 @@ 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;
 		trace_nfsd_deleg_read(&dp->dl_stid.sc_stateid);
 	}
 	nfs4_put_stid(&dp->dl_stid);
@@ -10447,7 +10443,7 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate,
 /**
  * nfsd4_vet_deleg_time - vet and set the timespec for a delegated timestamp update
  * @req: timestamp from the client
- * @orig: original timestamp in the inode
+ * @cur: current timestamp in the inode
  * @now: current time
  *
  * Given a timestamp from the client response, check it against the
@@ -10455,15 +10451,17 @@ nfsd4_get_writestateid(struct nfsd4_compound_state *cstate,
  * if the inode's timestamp needs to be updated, and false otherwise.
  * @req may also be changed if the timestamp needs to be clamped.
  */
-bool nfsd4_vet_deleg_time(struct timespec64 *req, const struct timespec64 *orig,
-			  const struct timespec64 *now)
+static bool nfsd4_vet_deleg_time(struct timespec64 *req,
+				 const struct timespec64 *cur,
+				 const struct timespec64 *now)
 {
-
 	/*
-	 * "When the time presented is before the original time, then the
-	 *  update is ignored." Also no need to update if there is no change.
+	 * RFC 9754 has the server ignore a time that is before the original
+	 * one. Compare against the value in the inode rather than the original:
+	 * an earlier CB_GETATTR or SETATTR may have moved it, and a report that
+	 * does not advance it is stale.
 	 */
-	if (timespec64_compare(req, orig) <= 0)
+	if (timespec64_compare(req, cur) <= 0)
 		return false;
 
 	/*
@@ -10484,30 +10482,45 @@ static int cb_getattr_update_times(struct dentry *dentry, struct nfs4_delegation
 	struct iattr attrs = { };
 	int ret;
 
+	/*
+	 * Take the inode lock first, so that a setattr from a delegation break
+	 * cannot land between the comparison and notify_change(). The atime can
+	 * still move under us: touch_atime() takes no lock, and FMODE_NOCMTIME
+	 * does not cover it.
+	 */
+	inode_lock(inode);
+
 	if (deleg_attrs_deleg(dp->dl_type)) {
 		struct timespec64 now = current_time(inode);
+		struct timespec64 atime = inode_get_atime(inode);
+		struct timespec64 mtime = inode_get_mtime(inode);
 
 		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))
+		if (nfsd4_vet_deleg_time(&attrs.ia_atime, &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;
+		/*
+		 * A change to the mtime must show in the ctime.
+		 * inode_set_ctime_deleg() takes the mtime when it advances the
+		 * ctime, and stamps the current time otherwise.
+		 */
+		if (nfsd4_vet_deleg_time(&attrs.ia_mtime, &mtime, &now)) {
+			attrs.ia_valid |= ATTR_MTIME | ATTR_MTIME_SET |
+					  ATTR_CTIME | ATTR_CTIME_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;
 		}
 	} else {
 		attrs.ia_valid |= ATTR_MTIME | ATTR_CTIME;
 	}
 
-	if (!attrs.ia_valid)
+	if (!attrs.ia_valid) {
+		inode_unlock(inode);
 		return 0;
+	}
 
 	attrs.ia_valid |= ATTR_DELEG;
-	inode_lock(inode);
 	ret = notify_change(&nop_mnt_idmap, dentry, &attrs, NULL);
 	inode_unlock(inode);
 	return ret;
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index cd9294f024bb..e7fe2af45f53 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -330,11 +330,6 @@ struct nfs4_delegation {
 		struct nfsd4_cb_notify	dl_cb_notify;
 	};
 
-	/* For delegated timestamps */
-	struct timespec64	dl_atime;
-	struct timespec64	dl_mtime;
-	struct timespec64	dl_ctime;
-
 	/* For dir delegations */
 	u32			dl_notify_mask;
 	u32			dl_child_attrs[2];
@@ -357,9 +352,6 @@ static inline bool deleg_attrs_deleg(u32 dl_type)
 	       dl_type == OPEN_DELEGATE_WRITE_ATTRS_DELEG;
 }
 
-bool nfsd4_vet_deleg_time(struct timespec64 *cb, const struct timespec64 *orig,
-			  const struct timespec64 *now);
-
 #define cb_to_delegation(cb) \
 	container_of(cb, struct nfs4_delegation, dl_recall)
 

-- 
2.55.0

Re: [PATCH v2 0/3] fs/nfsd: accept a backdated timestamp from a delegation holder

[Chuck Lever]

On Wed, Sep 2, 2026, at 2:54 PM, Jeff Layton wrote:
> The main problem is that an attribute delegation has strict rules about
> updates can be done, and that prevents applications that backdate
> timestamps on files after writing them, (e.g. tar -x, cp -p, etc.) from
> setting them properly.
>
> We could fix this on the client by making it always issue an RPC for the
> SETATTR, but that would harm performance in these common use-cases.
> This set allows the server to accept some of these backdated mtime
> updates, by changing how the ctime is handled.
>
> It'd be nice to see this in v7.4.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> Changes in v2:
> - Split the change into three patches
> - Stamp now in inode_set_ctime_deleg() for a stale update, so nfsd need
>   not compare
> - Drop a SETATTR report that matches the inode, so an untouched file
>   keeps its change attribute
> - Set dl_setattr only on the branch that carries a c/mtime update
> - Compare against the inode, not the grant-time snapshots (Chuck)
> - Set the ctime whenever a CB_GETATTR moves the mtime
> - Take i_rwsem before the CB_GETATTR comparison
> - Drop the unused dl_atime, dl_mtime and dl_ctime, and make
>   nfsd4_vet_deleg_time() static
> - Link to v1: 
> https://lore.kernel.org/r/20260901-delegts-v1-1-9937f7ee4370@kernel.org
>
> ---
> Jeff Layton (3):
>       fs: stamp the current time for a stale delegated ctime update
>       nfsd: accept a backdated timestamp from a delegation holder
>       nfsd: compare CB_GETATTR times against the inode
>
>  fs/inode.c          | 17 +++++++++++----
>  fs/nfsd/nfs4proc.c  | 63 +++++++++++++++++++++++++++++++++++++++++++----------
>  fs/nfsd/nfs4state.c | 49 ++++++++++++++++++++++++++---------------
>  fs/nfsd/state.h     |  8 -------
>  4 files changed, 95 insertions(+), 42 deletions(-)
> ---
> base-commit: ac579868af0c900d8fd9c8cfe9e10db46ccc5a75
> change-id: 20260901-delegts-e91a648f1427
>
> Best regards,
> -- 
> Jeff Layton <jlayton@kernel.org>

Since the series modifies fs/inode.c, Christian will want to take it
through the VFS tree.

Acked-by: Chuck Lever <cel@kernel.org>


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)

Re: [PATCH v2 0/3] fs/nfsd: accept a backdated timestamp from a delegation holder

[Christian Brauner]

On Wed, Sep 02, 2026 at 08:03:53PM -0400, Chuck Lever wrote:
> 
> 
> On Wed, Sep 2, 2026, at 2:54 PM, Jeff Layton wrote:
> > The main problem is that an attribute delegation has strict rules about
> > updates can be done, and that prevents applications that backdate
> > timestamps on files after writing them, (e.g. tar -x, cp -p, etc.) from
> > setting them properly.
> >
> > We could fix this on the client by making it always issue an RPC for the
> > SETATTR, but that would harm performance in these common use-cases.
> > This set allows the server to accept some of these backdated mtime
> > updates, by changing how the ctime is handled.
> >
> > It'd be nice to see this in v7.4.
> >
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > Changes in v2:
> > - Split the change into three patches
> > - Stamp now in inode_set_ctime_deleg() for a stale update, so nfsd need
> >   not compare
> > - Drop a SETATTR report that matches the inode, so an untouched file
> >   keeps its change attribute
> > - Set dl_setattr only on the branch that carries a c/mtime update
> > - Compare against the inode, not the grant-time snapshots (Chuck)
> > - Set the ctime whenever a CB_GETATTR moves the mtime
> > - Take i_rwsem before the CB_GETATTR comparison
> > - Drop the unused dl_atime, dl_mtime and dl_ctime, and make
> >   nfsd4_vet_deleg_time() static
> > - Link to v1: 
> > https://lore.kernel.org/r/20260901-delegts-v1-1-9937f7ee4370@kernel.org
> >
> > ---
> > Jeff Layton (3):
> >       fs: stamp the current time for a stale delegated ctime update
> >       nfsd: accept a backdated timestamp from a delegation holder
> >       nfsd: compare CB_GETATTR times against the inode
> >
> >  fs/inode.c          | 17 +++++++++++----
> >  fs/nfsd/nfs4proc.c  | 63 +++++++++++++++++++++++++++++++++++++++++++----------
> >  fs/nfsd/nfs4state.c | 49 ++++++++++++++++++++++++++---------------
> >  fs/nfsd/state.h     |  8 -------
> >  4 files changed, 95 insertions(+), 42 deletions(-)
> > ---
> > base-commit: ac579868af0c900d8fd9c8cfe9e10db46ccc5a75
> > change-id: 20260901-delegts-e91a648f1427
> >
> > Best regards,
> > -- 
> > Jeff Layton <jlayton@kernel.org>
> 
> Since the series modifies fs/inode.c, Christian will want to take it
> through the VFS tree.
> 
> Acked-by: Chuck Lever <cel@kernel.org>

Thanks!

Re: [PATCH v2 0/3] fs/nfsd: accept a backdated timestamp from a delegation holder

[Christian Brauner]

> The main problem is that an attribute delegation has strict rules about
> updates can be done, and that prevents applications that backdate
> timestamps on files after writing them, (e.g. tar -x, cp -p, etc.) from
> setting them properly.
> 
> We could fix this on the client by making it always issue an RPC for the
> SETATTR, but that would harm performance in these common use-cases.
> This set allows the server to accept some of these backdated mtime
> updates, by changing how the ctime is handled.
> 
> It'd be nice to see this in v7.4.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---

There's a bunch of Sashiko comments outstanding, Jeff. Let me know, if I
can ignore them.

-- 

Re: [PATCH v2 0/3] fs/nfsd: accept a backdated timestamp from a delegation holder

[Jeff Layton]

On Fri, 2026-09-04 at 12:50 +0200, Christian Brauner wrote:
> > The main problem is that an attribute delegation has strict rules about
> > updates can be done, and that prevents applications that backdate
> > timestamps on files after writing them, (e.g. tar -x, cp -p, etc.) from
> > setting them properly.
> > 
> > We could fix this on the client by making it always issue an RPC for the
> > SETATTR, but that would harm performance in these common use-cases.
> > This set allows the server to accept some of these backdated mtime
> > updates, by changing how the ctime is handled.
> > 
> > It'd be nice to see this in v7.4.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> 
> There's a bunch of Sashiko comments outstanding, Jeff. Let me know, if I
> can ignore them.


At least one of them looks valid. I'll plan to send a v3.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>