There are a couple of problems with delegated timestamp updates via
setattr:
1/ the ia_ctime is always clobbered by notify_change(), so setting the
ia_ctime to the same value as the ia_mtime in nfsd4_decode_fattr4()
doesn't work.
2/ while it does test the ctime's validity vs. the existing ctime and
current_time(), the same is not done for the atime or mtime. The spec
requires this.
Add a new setattr_copy_delegts() function that handles updating the
timestamps whenever ATTR_DELEG is set. For both atime and mtime,
validate and clamp the value to current_time(), and then set it. If the
mtime gets updated, also update the ctime.
Fixes: 7f2c86cba3c5 ("fs: handle delegated timestamps in setattr_copy_mgtime")
Signed-off-by: Jeff Layton <jlay...@kernel.org>
---
fs/attr.c | 52 ++++++++++++++++++++++++++++++++++++++--------------
fs/nfsd/nfs4xdr.c | 4 +---
2 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/fs/attr.c b/fs/attr.c
index
9caf63d20d03e86c535e9c8c91d49c2a34d34b7a..3e636943d26a36aeeed0ff8b428b6dd3e63f8dde
100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -287,14 +287,7 @@ static void setattr_copy_mgtime(struct inode *inode, const
struct iattr *attr)
struct timespec64 now;
if (ia_valid & ATTR_CTIME) {
- /*
- * In the case of an update for a write delegation, we must
respect
- * the value in ia_ctime and not use the current time.
- */
- if (ia_valid & ATTR_DELEG)
- now = inode_set_ctime_deleg(inode, attr->ia_ctime);
- else
- now = inode_set_ctime_current(inode);
+ now = inode_set_ctime_current(inode);
} else {
/* If ATTR_CTIME isn't set, then ATTR_MTIME shouldn't be
either. */
WARN_ON_ONCE(ia_valid & ATTR_MTIME);
@@ -312,6 +305,39 @@ static void setattr_copy_mgtime(struct inode *inode, const
struct iattr *attr)
inode_set_mtime_to_ts(inode, now);
}
+/*
+ * Skip update if new value is older than existing time. Clamp
+ * to current_time() if it's in the future.
+ */
+static void setattr_copy_delegts(struct inode *inode, const struct iattr *attr)
+{
+ struct timespec64 now = current_time(inode);
+ unsigned int ia_valid = attr->ia_valid;
+
+ if (ia_valid & ATTR_MTIME) {
+ struct timespec64 cur = inode_get_mtime(inode);
+
+ if (timespec64_compare(&attr->ia_mtime, &cur) > 0) {
+ if (timespec64_compare(&attr->ia_mtime, &now) > 0)
+ inode_set_mtime_to_ts(inode, now);
+ else
+ inode_set_mtime_to_ts(inode, attr->ia_mtime);
+ inode_set_ctime_deleg(inode, attr->ia_mtime);
+ }
+ }
+
+ if (ia_valid & ATTR_ATIME) {
+ struct timespec64 cur = inode_get_atime(inode);
+
+ if (timespec64_compare(&attr->ia_atime, &cur) > 0) {
+ if (timespec64_compare(&attr->ia_atime, &now) > 0)
+ inode_set_atime_to_ts(inode, now);
+ else
+ inode_set_atime_to_ts(inode, attr->ia_atime);
+ }
+ }
+}
+
/**
* setattr_copy - copy simple metadata updates into the generic inode
* @idmap: idmap of the mount the inode was found from
@@ -352,6 +378,8 @@ void setattr_copy(struct mnt_idmap *idmap, struct inode
*inode,
inode->i_mode = mode;
}
+ if (ia_valid & ATTR_DELEG)
+ return setattr_copy_delegts(inode, attr);
if (is_mgtime(inode))
return setattr_copy_mgtime(inode, attr);
@@ -359,12 +387,8 @@ void setattr_copy(struct mnt_idmap *idmap, struct inode
*inode,
inode_set_atime_to_ts(inode, attr->ia_atime);
if (ia_valid & ATTR_MTIME)
inode_set_mtime_to_ts(inode, attr->ia_mtime);
- if (ia_valid & ATTR_CTIME) {
- if (ia_valid & ATTR_DELEG)
- inode_set_ctime_deleg(inode, attr->ia_ctime);
- else
- inode_set_ctime_to_ts(inode, attr->ia_ctime);
- }
+ if (ia_valid & ATTR_CTIME)
+ inode_set_ctime_to_ts(inode, attr->ia_ctime);
}
EXPORT_SYMBOL(setattr_copy);
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index
8b68f74a8cf08c6aa1305a2a3093467656085e4a..e6899a3502332d686138abee2284c87fc7fbc0ae
100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -537,9 +537,7 @@ nfsd4_decode_fattr4(struct nfsd4_compoundargs *argp, u32
*bmval, u32 bmlen,
return nfserr_bad_xdr;
iattr->ia_mtime.tv_sec = modify.seconds;
iattr->ia_mtime.tv_nsec = modify.nseconds;
- iattr->ia_ctime.tv_sec = modify.seconds;
- iattr->ia_ctime.tv_nsec = modify.seconds;
- iattr->ia_valid |= ATTR_CTIME | ATTR_MTIME | ATTR_MTIME_SET |
ATTR_DELEG;
+ iattr->ia_valid |= ATTR_MTIME | ATTR_MTIME_SET | ATTR_DELEG;
}
/* request sanity: did attrlist4 contain the expected number of words?
*/
--
2.50.1
