[PATCH] fs: annotate inode timestamp accessors

[PATCH] fs: annotate inode timestamp accessors

[Yu Peng]

syzbot reported a KCSAN race between fill_mg_cmtime() and
inode_set_ctime_to_ts() on inode->i_ctime_{sec,nsec}.

stat/getattr can sample inode timestamps while update paths store new
values concurrently, so KCSAN can report benign races on these fields.

Annotate the timestamp accessors with READ_ONCE()/WRITE_ONCE(), and use
the ctime accessor for the remaining ctime loads.  This avoids the KCSAN
reports without changing timestamp semantics.

Fixes: 4e40eff0b573 ("fs: add infrastructure for multigrain timestamps")
Reported-by: syzbot+8b3bd9f8a06658479d4a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8b3bd9f8a06658479d4a
Signed-off-by: Yu Peng <pengyu@kylinos.cn>
---
 fs/inode.c         | 18 +++++++++---------
 fs/stat.c          |  2 +-
 include/linux/fs.h | 20 ++++++++++----------
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 31c5b9ee3a81..95e981b4e19c 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2833,8 +2833,8 @@ struct timespec64 inode_set_ctime_to_ts(struct inode *inode, struct timespec64 t
 {
 	trace_inode_set_ctime_to_ts(inode, &ts);
 	set_normalized_timespec64(&ts, ts.tv_sec, ts.tv_nsec);
-	inode->i_ctime_sec = ts.tv_sec;
-	inode->i_ctime_nsec = ts.tv_nsec;
+	WRITE_ONCE(inode->i_ctime_sec, ts.tv_sec);
+	WRITE_ONCE(inode->i_ctime_nsec, ts.tv_nsec);
 	return ts;
 }
 EXPORT_SYMBOL(inode_set_ctime_to_ts);
@@ -2908,7 +2908,7 @@ struct timespec64 inode_set_ctime_current(struct inode *inode)
 	 */
 	cns = smp_load_acquire(&inode->i_ctime_nsec);
 	if (cns & I_CTIME_QUERIED) {
-		struct timespec64 ctime = { .tv_sec = inode->i_ctime_sec,
+		struct timespec64 ctime = { .tv_sec = inode_get_ctime_sec(inode),
 					    .tv_nsec = cns & ~I_CTIME_QUERIED };
 
 		if (timespec64_compare(&now, &ctime) <= 0) {
@@ -2920,7 +2920,7 @@ struct timespec64 inode_set_ctime_current(struct inode *inode)
 	mgtime_counter_inc(mg_ctime_updates);
 
 	/* No need to cmpxchg if it's exactly the same */
-	if (cns == now.tv_nsec && inode->i_ctime_sec == now.tv_sec) {
+	if (cns == now.tv_nsec && inode_get_ctime_sec(inode) == now.tv_sec) {
 		trace_ctime_xchg_skip(inode, &now);
 		goto out;
 	}
@@ -2929,7 +2929,7 @@ struct timespec64 inode_set_ctime_current(struct inode *inode)
 	/* Try to swap the nsec value into place. */
 	if (try_cmpxchg(&inode->i_ctime_nsec, &cur, now.tv_nsec)) {
 		/* If swap occurred, then we're (mostly) done */
-		inode->i_ctime_sec = now.tv_sec;
+		WRITE_ONCE(inode->i_ctime_sec, now.tv_sec);
 		trace_ctime_ns_xchg(inode, cns, now.tv_nsec, cur);
 		mgtime_counter_inc(mg_ctime_swaps);
 	} else {
@@ -2944,7 +2944,7 @@ struct timespec64 inode_set_ctime_current(struct inode *inode)
 			goto retry;
 		}
 		/* Otherwise, keep the existing ctime */
-		now.tv_sec = inode->i_ctime_sec;
+		now.tv_sec = inode_get_ctime_sec(inode);
 		now.tv_nsec = cur & ~I_CTIME_QUERIED;
 	}
 out:
@@ -2977,7 +2977,7 @@ struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 u
 	/* pairs with try_cmpxchg below */
 	cur = smp_load_acquire(&inode->i_ctime_nsec);
 	cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED;
-	cur_ts.tv_sec = inode->i_ctime_sec;
+	cur_ts.tv_sec = inode_get_ctime_sec(inode);
 
 	/* If the update is older than the existing value, skip it. */
 	if (timespec64_compare(&update, &cur_ts) <= 0)
@@ -3003,7 +3003,7 @@ struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 u
 retry:
 	old = cur;
 	if (try_cmpxchg(&inode->i_ctime_nsec, &cur, update.tv_nsec)) {
-		inode->i_ctime_sec = update.tv_sec;
+		WRITE_ONCE(inode->i_ctime_sec, update.tv_sec);
 		mgtime_counter_inc(mg_ctime_swaps);
 		return update;
 	}
@@ -3019,7 +3019,7 @@ struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 u
 		goto retry;
 
 	/* Otherwise, it was a new timestamp. */
-	cur_ts.tv_sec = inode->i_ctime_sec;
+	cur_ts.tv_sec = inode_get_ctime_sec(inode);
 	cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED;
 	return cur_ts;
 }
diff --git a/fs/stat.c b/fs/stat.c
index 89909746bed1..c461c3054234 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -53,7 +53,7 @@ void fill_mg_cmtime(struct kstat *stat, u32 request_mask, struct inode *inode)
 	}
 
 	stat->mtime = inode_get_mtime(inode);
-	stat->ctime.tv_sec = inode->i_ctime_sec;
+	stat->ctime.tv_sec = inode_get_ctime_sec(inode);
 	stat->ctime.tv_nsec = (u32)atomic_read(pcn);
 	if (!(stat->ctime.tv_nsec & I_CTIME_QUERIED))
 		stat->ctime.tv_nsec = ((u32)atomic_fetch_or(I_CTIME_QUERIED, pcn));
diff --git a/include/linux/fs.h b/include/linux/fs.h
index d10897b3a1e3..e5b97e324db1 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1598,12 +1598,12 @@ struct timespec64 inode_set_ctime_deleg(struct inode *inode,
 
 static inline time64_t inode_get_atime_sec(const struct inode *inode)
 {
-	return inode->i_atime_sec;
+	return READ_ONCE(inode->i_atime_sec);
 }
 
 static inline long inode_get_atime_nsec(const struct inode *inode)
 {
-	return inode->i_atime_nsec;
+	return READ_ONCE(inode->i_atime_nsec);
 }
 
 static inline struct timespec64 inode_get_atime(const struct inode *inode)
@@ -1617,8 +1617,8 @@ static inline struct timespec64 inode_get_atime(const struct inode *inode)
 static inline struct timespec64 inode_set_atime_to_ts(struct inode *inode,
 						      struct timespec64 ts)
 {
-	inode->i_atime_sec = ts.tv_sec;
-	inode->i_atime_nsec = ts.tv_nsec;
+	WRITE_ONCE(inode->i_atime_sec, ts.tv_sec);
+	WRITE_ONCE(inode->i_atime_nsec, ts.tv_nsec);
 	return ts;
 }
 
@@ -1633,12 +1633,12 @@ static inline struct timespec64 inode_set_atime(struct inode *inode,
 
 static inline time64_t inode_get_mtime_sec(const struct inode *inode)
 {
-	return inode->i_mtime_sec;
+	return READ_ONCE(inode->i_mtime_sec);
 }
 
 static inline long inode_get_mtime_nsec(const struct inode *inode)
 {
-	return inode->i_mtime_nsec;
+	return READ_ONCE(inode->i_mtime_nsec);
 }
 
 static inline struct timespec64 inode_get_mtime(const struct inode *inode)
@@ -1651,8 +1651,8 @@ static inline struct timespec64 inode_get_mtime(const struct inode *inode)
 static inline struct timespec64 inode_set_mtime_to_ts(struct inode *inode,
 						      struct timespec64 ts)
 {
-	inode->i_mtime_sec = ts.tv_sec;
-	inode->i_mtime_nsec = ts.tv_nsec;
+	WRITE_ONCE(inode->i_mtime_sec, ts.tv_sec);
+	WRITE_ONCE(inode->i_mtime_nsec, ts.tv_nsec);
 	return ts;
 }
 
@@ -1677,12 +1677,12 @@ static inline struct timespec64 inode_set_mtime(struct inode *inode,
 
 static inline time64_t inode_get_ctime_sec(const struct inode *inode)
 {
-	return inode->i_ctime_sec;
+	return READ_ONCE(inode->i_ctime_sec);
 }
 
 static inline long inode_get_ctime_nsec(const struct inode *inode)
 {
-	return inode->i_ctime_nsec & ~I_CTIME_QUERIED;
+	return READ_ONCE(inode->i_ctime_nsec) & ~I_CTIME_QUERIED;
 }
 
 static inline struct timespec64 inode_get_ctime(const struct inode *inode)
-- 
2.43.0

Re: [PATCH] fs: annotate inode timestamp accessors

[Jeff Layton]

On Wed, 2026-07-08 at 16:02 +0800, Yu Peng wrote:
> syzbot reported a KCSAN race between fill_mg_cmtime() and
> inode_set_ctime_to_ts() on inode->i_ctime_{sec,nsec}.
> 
> stat/getattr can sample inode timestamps while update paths store new
> values concurrently, so KCSAN can report benign races on these fields.
> 
> Annotate the timestamp accessors with READ_ONCE()/WRITE_ONCE(), and use
> the ctime accessor for the remaining ctime loads.  This avoids the KCSAN
> reports without changing timestamp semantics.
> 
> Fixes: 4e40eff0b573 ("fs: add infrastructure for multigrain timestamps")
> Reported-by: syzbot+8b3bd9f8a06658479d4a@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=8b3bd9f8a06658479d4a
> Signed-off-by: Yu Peng <pengyu@kylinos.cn>
> ---
>  fs/inode.c         | 18 +++++++++---------
>  fs/stat.c          |  2 +-
>  include/linux/fs.h | 20 ++++++++++----------
>  3 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index 31c5b9ee3a81..95e981b4e19c 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2833,8 +2833,8 @@ struct timespec64 inode_set_ctime_to_ts(struct inode *inode, struct timespec64 t
>  {
>  	trace_inode_set_ctime_to_ts(inode, &ts);
>  	set_normalized_timespec64(&ts, ts.tv_sec, ts.tv_nsec);
> -	inode->i_ctime_sec = ts.tv_sec;
> -	inode->i_ctime_nsec = ts.tv_nsec;
> +	WRITE_ONCE(inode->i_ctime_sec, ts.tv_sec);
> +	WRITE_ONCE(inode->i_ctime_nsec, ts.tv_nsec);
>  	return ts;
>  }
>  EXPORT_SYMBOL(inode_set_ctime_to_ts);
> @@ -2908,7 +2908,7 @@ struct timespec64 inode_set_ctime_current(struct inode *inode)
>  	 */
>  	cns = smp_load_acquire(&inode->i_ctime_nsec);
>  	if (cns & I_CTIME_QUERIED) {
> -		struct timespec64 ctime = { .tv_sec = inode->i_ctime_sec,
> +		struct timespec64 ctime = { .tv_sec = inode_get_ctime_sec(inode),
>  					    .tv_nsec = cns & ~I_CTIME_QUERIED };
>  
>  		if (timespec64_compare(&now, &ctime) <= 0) {
> @@ -2920,7 +2920,7 @@ struct timespec64 inode_set_ctime_current(struct inode *inode)
>  	mgtime_counter_inc(mg_ctime_updates);
>  
>  	/* No need to cmpxchg if it's exactly the same */
> -	if (cns == now.tv_nsec && inode->i_ctime_sec == now.tv_sec) {
> +	if (cns == now.tv_nsec && inode_get_ctime_sec(inode) == now.tv_sec) {
>  		trace_ctime_xchg_skip(inode, &now);
>  		goto out;
>  	}
> @@ -2929,7 +2929,7 @@ struct timespec64 inode_set_ctime_current(struct inode *inode)
>  	/* Try to swap the nsec value into place. */
>  	if (try_cmpxchg(&inode->i_ctime_nsec, &cur, now.tv_nsec)) {
>  		/* If swap occurred, then we're (mostly) done */
> -		inode->i_ctime_sec = now.tv_sec;
> +		WRITE_ONCE(inode->i_ctime_sec, now.tv_sec);
>  		trace_ctime_ns_xchg(inode, cns, now.tv_nsec, cur);
>  		mgtime_counter_inc(mg_ctime_swaps);
>  	} else {
> @@ -2944,7 +2944,7 @@ struct timespec64 inode_set_ctime_current(struct inode *inode)
>  			goto retry;
>  		}
>  		/* Otherwise, keep the existing ctime */
> -		now.tv_sec = inode->i_ctime_sec;
> +		now.tv_sec = inode_get_ctime_sec(inode);
>  		now.tv_nsec = cur & ~I_CTIME_QUERIED;
>  	}
>  out:
> @@ -2977,7 +2977,7 @@ struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 u
>  	/* pairs with try_cmpxchg below */
>  	cur = smp_load_acquire(&inode->i_ctime_nsec);
>  	cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED;
> -	cur_ts.tv_sec = inode->i_ctime_sec;
> +	cur_ts.tv_sec = inode_get_ctime_sec(inode);
>  
>  	/* If the update is older than the existing value, skip it. */
>  	if (timespec64_compare(&update, &cur_ts) <= 0)
> @@ -3003,7 +3003,7 @@ struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 u
>  retry:
>  	old = cur;
>  	if (try_cmpxchg(&inode->i_ctime_nsec, &cur, update.tv_nsec)) {
> -		inode->i_ctime_sec = update.tv_sec;
> +		WRITE_ONCE(inode->i_ctime_sec, update.tv_sec);
>  		mgtime_counter_inc(mg_ctime_swaps);
>  		return update;
>  	}
> @@ -3019,7 +3019,7 @@ struct timespec64 inode_set_ctime_deleg(struct inode *inode, struct timespec64 u
>  		goto retry;
>  
>  	/* Otherwise, it was a new timestamp. */
> -	cur_ts.tv_sec = inode->i_ctime_sec;
> +	cur_ts.tv_sec = inode_get_ctime_sec(inode);
>  	cur_ts.tv_nsec = cur & ~I_CTIME_QUERIED;
>  	return cur_ts;
>  }
> diff --git a/fs/stat.c b/fs/stat.c
> index 89909746bed1..c461c3054234 100644
> --- a/fs/stat.c
> +++ b/fs/stat.c
> @@ -53,7 +53,7 @@ void fill_mg_cmtime(struct kstat *stat, u32 request_mask, struct inode *inode)
>  	}
>  
>  	stat->mtime = inode_get_mtime(inode);
> -	stat->ctime.tv_sec = inode->i_ctime_sec;
> +	stat->ctime.tv_sec = inode_get_ctime_sec(inode);
>  	stat->ctime.tv_nsec = (u32)atomic_read(pcn);
>  	if (!(stat->ctime.tv_nsec & I_CTIME_QUERIED))
>  		stat->ctime.tv_nsec = ((u32)atomic_fetch_or(I_CTIME_QUERIED, pcn));
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index d10897b3a1e3..e5b97e324db1 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1598,12 +1598,12 @@ struct timespec64 inode_set_ctime_deleg(struct inode *inode,
>  
>  static inline time64_t inode_get_atime_sec(const struct inode *inode)
>  {
> -	return inode->i_atime_sec;
> +	return READ_ONCE(inode->i_atime_sec);
>  }
>  
>  static inline long inode_get_atime_nsec(const struct inode *inode)
>  {
> -	return inode->i_atime_nsec;
> +	return READ_ONCE(inode->i_atime_nsec);
>  }
>  
>  static inline struct timespec64 inode_get_atime(const struct inode *inode)
> @@ -1617,8 +1617,8 @@ static inline struct timespec64 inode_get_atime(const struct inode *inode)
>  static inline struct timespec64 inode_set_atime_to_ts(struct inode *inode,
>  						      struct timespec64 ts)
>  {
> -	inode->i_atime_sec = ts.tv_sec;
> -	inode->i_atime_nsec = ts.tv_nsec;
> +	WRITE_ONCE(inode->i_atime_sec, ts.tv_sec);
> +	WRITE_ONCE(inode->i_atime_nsec, ts.tv_nsec);
>  	return ts;
>  }
>  
> @@ -1633,12 +1633,12 @@ static inline struct timespec64 inode_set_atime(struct inode *inode,
>  
>  static inline time64_t inode_get_mtime_sec(const struct inode *inode)
>  {
> -	return inode->i_mtime_sec;
> +	return READ_ONCE(inode->i_mtime_sec);
>  }
>  
>  static inline long inode_get_mtime_nsec(const struct inode *inode)
>  {
> -	return inode->i_mtime_nsec;
> +	return READ_ONCE(inode->i_mtime_nsec);
>  }
>  
>  static inline struct timespec64 inode_get_mtime(const struct inode *inode)
> @@ -1651,8 +1651,8 @@ static inline struct timespec64 inode_get_mtime(const struct inode *inode)
>  static inline struct timespec64 inode_set_mtime_to_ts(struct inode *inode,
>  						      struct timespec64 ts)
>  {
> -	inode->i_mtime_sec = ts.tv_sec;
> -	inode->i_mtime_nsec = ts.tv_nsec;
> +	WRITE_ONCE(inode->i_mtime_sec, ts.tv_sec);
> +	WRITE_ONCE(inode->i_mtime_nsec, ts.tv_nsec);
>  	return ts;
>  }
>  
> @@ -1677,12 +1677,12 @@ static inline struct timespec64 inode_set_mtime(struct inode *inode,
>  
>  static inline time64_t inode_get_ctime_sec(const struct inode *inode)
>  {
> -	return inode->i_ctime_sec;
> +	return READ_ONCE(inode->i_ctime_sec);
>  }
>  
>  static inline long inode_get_ctime_nsec(const struct inode *inode)
>  {
> -	return inode->i_ctime_nsec & ~I_CTIME_QUERIED;
> +	return READ_ONCE(inode->i_ctime_nsec) & ~I_CTIME_QUERIED;
>  }
>  
>  static inline struct timespec64 inode_get_ctime(const struct inode *inode)

Nice catch. We could also do this with data_race(), I suppose, which
would allow the compiler to do a bit more optimization, but I don't
think it'll make any significant difference to the generated code.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH] fs: annotate inode timestamp accessors

[Christian Brauner]

On Wed, 08 Jul 2026 16:02:32 +0800, Yu Peng wrote:
> fs: annotate inode timestamp accessors

Applied to the vfs-7.3.misc branch of the vfs/vfs.git tree.
Patches in the vfs-7.3.misc 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-7.3.misc

[1/1] fs: annotate inode timestamp accessors
      https://git.kernel.org/vfs/vfs/c/3bd226eeef39