linux-kernel - [PATCH v3] fs: Fix data race in inode_set_ctime_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20241122130642.460929-1-zhenghaoran@buaa.edu.cn>
Date: Fri, 22 Nov 2024 21:06:42 +0800
From: Hao-ran Zheng <zhenghaoran@...a.edu.cn>
To: viro@...iv.linux.org.uk,
	brauner@...nel.org,
	jack@...e.cz,
	linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Cc: baijiaju1990@...il.com,
	zhenghaoran@...a.edu.cn,
	21371365@...a.edu.cn
Subject: [PATCH v3] fs: Fix data race in inode_set_ctime_to_ts

A data race may occur when the function `inode_set_ctime_to_ts()` and
the function `inode_get_ctime_sec()` are executed concurrently. When
two threads call `aio_read` and `aio_write` respectively, they will
be distributed to the read and write functions of the corresponding
file system respectively. Taking the btrfs file system as an example,
the `btrfs_file_read_iter` and `btrfs_file_write_iter` functions are
finally called. These two functions created a data race when they
finally called `inode_get_ctime_sec()` and `inode_set_ctime_to_ns()`.
The specific call stack that appears during testing is as follows:

============DATA_RACE============
btrfs_delayed_update_inode+0x1f61/0x7ce0 [btrfs]
btrfs_update_inode+0x45e/0xbb0 [btrfs]
btrfs_dirty_inode+0x2b8/0x530 [btrfs]
btrfs_update_time+0x1ad/0x230 [btrfs]
touch_atime+0x211/0x440
filemap_read+0x90f/0xa20
btrfs_file_read_iter+0xeb/0x580 [btrfs]
aio_read+0x275/0x3a0
io_submit_one+0xd22/0x1ce0
__se_sys_io_submit+0xb3/0x250
do_syscall_64+0xc1/0x190
entry_SYSCALL_64_after_hwframe+0x77/0x7f
============OTHER_INFO============
btrfs_write_check+0xa15/0x1390 [btrfs]
btrfs_buffered_write+0x52f/0x29d0 [btrfs]
btrfs_do_write_iter+0x53d/0x1590 [btrfs]
btrfs_file_write_iter+0x41/0x60 [btrfs]
aio_write+0x41e/0x5f0
io_submit_one+0xd42/0x1ce0
__se_sys_io_submit+0xb3/0x250
do_syscall_64+0xc1/0x190
entry_SYSCALL_64_after_hwframe+0x77/0x7f

To address this issue, it is recommended to add WRITE_ONCE
when writing the `inode->i_ctime_sec` variable.and add
READ_ONCE when reading in function `inode_get_ctime_sec()`
and `inode_get_ctime_nsec()`.

Signed-off-by: Hao-ran Zheng <zhenghaoran@...a.edu.cn>
---
V2 -> V3: Added READ_ONCE in inode_get_ctime_nsec() and addressed review comments
V1 -> V2: Added READ_ONCE in inode_get_ctime_sec()
---
 include/linux/fs.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3559446279c1..c18f9a9ee5e7 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1655,12 +1655,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;
+	return READ_ONCE(inode->i_ctime_nsec);
 }
 
 static inline struct timespec64 inode_get_ctime(const struct inode *inode)
@@ -1674,8 +1674,8 @@ static inline struct timespec64 inode_get_ctime(const struct inode *inode)
 static inline struct timespec64 inode_set_ctime_to_ts(struct inode *inode,
 						      struct timespec64 ts)
 {
-	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;
 }
 
-- 
2.34.1