2023-09-19 17:41:45

by Jeff Layton

[permalink] [raw]
Subject: [PATCH] fs: add bounds checking when updating ctime

During some discussion around another patch, Linus pointed out that
we don't want to skip updating the ctime unless the current coarse
time is in a reasonable range.

When updating the ctime on a multigrain filesystem, only keep the
current ctime if the coarse time is less than 2 jiffies earlier.

Suggested-by: Linus Torvalds <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
---
While we're still discussing whether to keep this series in, here's the
fix for the issue that Linus identified yesterday, basically that a
large clock jump backward could result in the m/ctime not being updated
properly.
---
fs/inode.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 54237f4242ff..f3d68e4b8df7 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2571,6 +2571,13 @@ struct timespec64 current_time(struct inode *inode)
}
EXPORT_SYMBOL(current_time);

+/*
+ * Coarse timer ticks happen (roughly) every jiffy. If we see a coarse time
+ * more than 2 jiffies earlier than the current ctime, then we need to
+ * update it. This is the max delta allowed (in ns).
+ */
+#define COARSE_TIME_MAX_DELTA (2 / HZ * NSEC_PER_SEC)
+
/**
* inode_set_ctime_current - set the ctime to current_time
* @inode: inode
@@ -2599,8 +2606,19 @@ struct timespec64 inode_set_ctime_current(struct inode *inode)
* existing ctime. Just keep the existing value if so.
*/
ctime.tv_sec = inode->__i_ctime.tv_sec;
- if (timespec64_compare(&ctime, &now) > 0)
- return ctime;
+ if (timespec64_compare(&ctime, &now) > 0) {
+ struct timespec64 limit = now;
+
+ /*
+ * If the current coarse-grained clock is earlier than
+ * it should be, then that's an indication that there
+ * may have been a backward clock jump, and that the
+ * update should not be skipped.
+ */
+ timespec64_add_ns(&limit, COARSE_TIME_MAX_DELTA);
+ if (timespec64_compare(&ctime, &limit) < 0)
+ return ctime;
+ }

/*
* Ctime updates are usually protected by the inode_lock, but

---
base-commit: 0b4cbb6924ecf459c12b2b5ff4370ae29a276fee
change-id: 20230919-ctime-14ad041893fb

Best regards,
--
Jeff Layton <[email protected]>


2023-09-20 09:57:40

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH] fs: add bounds checking when updating ctime

On Tue, 19 Sep 2023 13:35:11 -0400, Jeff Layton wrote:
> During some discussion around another patch, Linus pointed out that
> we don't want to skip updating the ctime unless the current coarse
> time is in a reasonable range.
>
> When updating the ctime on a multigrain filesystem, only keep the
> current ctime if the coarse time is less than 2 jiffies earlier.
>
> [...]

Applied to the vfs.ctime branch of the vfs/vfs.git tree.
Patches in the vfs.ctime branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.ctime

[1/1] fs: add bounds checking when updating ctime
https://git.kernel.org/vfs/vfs/c/0445adc637bc