[PATCH] nfsd: accept a backdated timestamp from a delegation holder

[PATCH] 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. It
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 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.
  The ctime never moves backwards.
- 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 delegation 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")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Assisted-by: Claude:claude-opus-5
---
We could fix this more correctly on the client by making it always issue
an RPC for a utimensat(), but I think that would hurt untar-type
workloads (which often backdate timestamps).
---
 fs/nfsd/nfs4proc.c | 47 +++++++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index bb74eef43938..fa3a43c20e95 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1292,9 +1292,22 @@ 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)
@@ -1302,21 +1315,23 @@ 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_ACCESS)
+		clamp_deleg_time(&iattr->ia_atime, &now);
 
 	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);
-		}
+		clamp_deleg_time(&iattr->ia_mtime, &now);
+
+		/*
+		 * The ctime must not move backwards. Carry the mtime into it
+		 * only when that advances it; otherwise clear ATTR_CTIME_SET so
+		 * that notify_change() stamps the ctime with the current time,
+		 * which is what a local utimensat() would do.
+		 */
+		iattr->ia_ctime = iattr->ia_mtime;
+		if (!nfsd4_vet_deleg_time(&iattr->ia_ctime, &dp->dl_ctime, &now))
+			iattr->ia_valid &= ~ATTR_CTIME_SET;
+
+		dp->dl_setattr = true;
 	}
 }
 

---
base-commit: cfebfd3db73d82143ac54b1b6c6dad1d13952d59
change-id: 20260901-delegts-e91a648f1427

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

Re: [PATCH] nfsd: accept a backdated timestamp from a delegation holder

[Chuck Lever]

On Tue, Sep 1, 2026, at 9:09 AM, Jeff Layton wrote:
> A client with an attribute delegation reports the file times in a
> SETATTR at DELEGRETURN. nfsd ignores a time that moves backwards. It
> 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 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.
>   The ctime never moves backwards.
> - 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 delegation 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")
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> Assisted-by: Claude:claude-opus-5


Hi Jeff, LLM-generated review suggested the below finding is a blocker.

> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index bb74eef43938..fa3a43c20e95 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c

[ ... ]

> @@ -1302,21 +1315,23 @@ 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_ACCESS)
> +		clamp_deleg_time(&iattr->ia_atime, &now);
>  
>  	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);
> -		}
> +		clamp_deleg_time(&iattr->ia_mtime, &now);
> +
> +		/*
> +		 * The ctime must not move backwards. Carry the mtime into it
> +		 * only when that advances it; otherwise clear ATTR_CTIME_SET so
> +		 * that notify_change() stamps the ctime with the current time,
> +		 * which is what a local utimensat() would do.
> +		 */
> +		iattr->ia_ctime = iattr->ia_mtime;
> +		if (!nfsd4_vet_deleg_time(&iattr->ia_ctime, &dp->dl_ctime, &now))
                                                             ^^^^^^^^^^^^

Is dp->dl_ctime the right value to compare against here?  It is sampled
once, when the delegation is granted, and nothing refreshes it after
that:

fs/nfsd/nfs4state.c:nfs4_open_delegation() {
    ...
	dp->dl_atime = stat.atime;
	dp->dl_ctime = stat.ctime;
	dp->dl_mtime = stat.mtime;
    ...
}

Is there anything that stops a client from sending more than one SETATTR
with TIME_DELEG_MODIFY while it holds the delegation?

Say the first one carries an mtime T5 that is later than dl_ctime.
nfsd4_vet_deleg_time() returns true, ATTR_CTIME_SET stays set, and the
inode ctime becomes T5.

If a second SETATTR then backdates the mtime to T3, where dl_ctime < T3 <
T5, the comparison is still against the grant-time dl_ctime.  It returns
true again, so ATTR_CTIME_SET stays set and T3 is carried into ia_ctime.

notify_change() -> setattr_copy() -> inode_set_ctime_deleg() then drops
that update, because T3 is older than the T5 already in the inode:

fs/inode.c:inode_set_ctime_deleg() {
    ...
	/* If the update is older than the existing value, skip it. */
	if (timespec64_compare(&update, &cur_ts) <= 0)
		return cur_ts;
    ...
}

The mtime moves back to T3 and the ctime stays at T5, so the ctime is
neither carried from the mtime nor stamped with the current time.  That
looks different from the comment just above it, and from the commit
message: "An mtime that moves backwards sets the ctime to the current
time."

Would comparing against the inode's current ctime work better here?  The
inode is already in hand for the current_time() call at the top of the
function.

> +
> +		dp->dl_setattr = true;
>  	}
>  }


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

Re: [PATCH] nfsd: accept a backdated timestamp from a delegation holder

[Jeff Layton]

On Tue, 2026-09-01 at 11:19 -0400, Chuck Lever wrote:
> On Tue, Sep 1, 2026, at 9:09 AM, Jeff Layton wrote:
> > A client with an attribute delegation reports the file times in a
> > SETATTR at DELEGRETURN. nfsd ignores a time that moves backwards. It
> > 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 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.
> >   The ctime never moves backwards.
> > - 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 delegation 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")
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > Assisted-by: Claude:claude-opus-5
> 
> 
> Hi Jeff, LLM-generated review suggested the below finding is a blocker.
> 
> > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > index bb74eef43938..fa3a43c20e95 100644
> > --- a/fs/nfsd/nfs4proc.c
> > +++ b/fs/nfsd/nfs4proc.c
> 
> [ ... ]
> 
> > @@ -1302,21 +1315,23 @@ 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_ACCESS)
> > +		clamp_deleg_time(&iattr->ia_atime, &now);
> >  
> >  	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);
> > -		}
> > +		clamp_deleg_time(&iattr->ia_mtime, &now);
> > +
> > +		/*
> > +		 * The ctime must not move backwards. Carry the mtime into it
> > +		 * only when that advances it; otherwise clear ATTR_CTIME_SET so
> > +		 * that notify_change() stamps the ctime with the current time,
> > +		 * which is what a local utimensat() would do.
> > +		 */
> > +		iattr->ia_ctime = iattr->ia_mtime;
> > +		if (!nfsd4_vet_deleg_time(&iattr->ia_ctime, &dp->dl_ctime, &now))
>                                                              ^^^^^^^^^^^^
> 
> Is dp->dl_ctime the right value to compare against here?  It is sampled
> once, when the delegation is granted, and nothing refreshes it after
> that:
> 
> fs/nfsd/nfs4state.c:nfs4_open_delegation() {
>     ...
> 	dp->dl_atime = stat.atime;
> 	dp->dl_ctime = stat.ctime;
> 	dp->dl_mtime = stat.mtime;
>     ...
> }
> 
> Is there anything that stops a client from sending more than one SETATTR
> with TIME_DELEG_MODIFY while it holds the delegation?
> 
> Say the first one carries an mtime T5 that is later than dl_ctime.
> nfsd4_vet_deleg_time() returns true, ATTR_CTIME_SET stays set, and the
> inode ctime becomes T5.
> 
> If a second SETATTR then backdates the mtime to T3, where dl_ctime < T3 <
> T5, the comparison is still against the grant-time dl_ctime.  It returns
> true again, so ATTR_CTIME_SET stays set and T3 is carried into ia_ctime.
> 
> notify_change() -> setattr_copy() -> inode_set_ctime_deleg() then drops
> that update, because T3 is older than the T5 already in the inode:
> 
> fs/inode.c:inode_set_ctime_deleg() {
>     ...
> 	/* If the update is older than the existing value, skip it. */
> 	if (timespec64_compare(&update, &cur_ts) <= 0)
> 		return cur_ts;
>     ...
> }
> 
> The mtime moves back to T3 and the ctime stays at T5, so the ctime is
> neither carried from the mtime nor stamped with the current time.  That
> looks different from the comment just above it, and from the commit
> message: "An mtime that moves backwards sets the ctime to the current
> time."
> 
> Would comparing against the inode's current ctime work better here?  The
> inode is already in hand for the current_time() call at the top of the
> function.
> 
> 

Yes, I think we do want to rework it to do that. Additionally, I
noticed another race condition that we ought to fix while we're in
here.

I'll be sending a v2, but I think we have to push some of this handling
into the VFS layer so that it's done under more consistent locking and
properly uses the mgtime infrastructure.

-- 
Jeff Layton <jlayton@kernel.org>