[PATCH 4/5] f2fs: fix to avoid race condition of f2fs_abort_atomic_write()

From: Chao Yu
Date: Sun Jan 08 2023 - 22:45:34 EST


Syzbot reports a kernel panic as below:

kernel BUG at fs/inode.c:1763!
RIP: 0010:iput+0x68/0x80 fs/inode.c:1763
Call Trace:
f2fs_abort_atomic_write+0xea/0x4f0 fs/f2fs/segment.c:196
f2fs_ioc_commit_atomic_write+0x19f/0x260 fs/f2fs/file.c:2157
__f2fs_ioctl+0x26f0/0xaaf0 fs/f2fs/file.c:4154
f2fs_ioctl+0x18e/0x220 fs/f2fs/file.c:4242
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:870 [inline]
__se_sys_ioctl fs/ioctl.c:856 [inline]
__x64_sys_ioctl+0x197/0x210 fs/ioctl.c:856
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd

The root cause is there may be race case in between f2fs_abort_atomic_write()
called from f2fs_file_flush() and f2fs_ioc_commit_atomic_write().

Thread A Thread B
- close
- close_fd
- filp_close
- f2fs_file_flush
- f2fs_ioctl
- f2fs_ioc_commit_atomic_write
- f2fs_abort_atomic_write
- iput(cow_inode)
- f2fs_evict_inode
- clear_inode
- inode->i_state = I_FREEING | I_CLEAR;
- f2fs_abort_atomic_write
- iput(cow_inode)
- BUG_ON(inode->i_state & I_CLEAR)
- cow_inode = NULL
- cow_inode = NULL

So let's cover f2fs_abort_atomic_write() with f2fs_inode_info.i_atomic_sem
lock to avoid this race condition.

Reported-by: syzbot+9b7be0f1263ed2dbfbba@xxxxxxxxxxxxxxxxxxxxxxxxx
Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
Signed-off-by: Chao Yu <chao@xxxxxxxxxx>
---
fs/f2fs/f2fs.h | 1 +
fs/f2fs/segment.c | 6 +++++-
fs/f2fs/super.c | 1 +
3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 70ed01aca6f6..fa41c0dad308 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -854,6 +854,7 @@ struct f2fs_inode_info {
struct extent_tree *extent_tree[NR_EXTENT_CACHES];
/* cached extent_tree entry */
struct inode *cow_inode; /* copy-on-write inode for atomic write */
+ struct f2fs_rwsem i_atomic_sem; /* protect atomic write context */

/* avoid racing between foreground op and gc */
struct f2fs_rwsem i_gc_rwsem[2];
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 5f7e42b355eb..245d7adef6c6 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -189,8 +189,10 @@ void f2fs_abort_atomic_write(struct inode *inode, bool clean)
{
struct f2fs_inode_info *fi = F2FS_I(inode);

+ f2fs_down_write(&fi->i_atomic_sem);
+
if (!f2fs_is_atomic_file(inode))
- return;
+ goto out_unlock;

clear_inode_flag(fi->cow_inode, FI_COW_FILE);
iput(fi->cow_inode);
@@ -208,6 +210,8 @@ void f2fs_abort_atomic_write(struct inode *inode, bool clean)
f2fs_i_size_write(inode, fi->original_i_size);
fi->original_i_size = 0;
}
+out_unlock:
+ f2fs_up_write(&fi->i_atomic_sem);
}

static int __replace_atomic_write_block(struct inode *inode, pgoff_t index,
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 0e8b3e27fa14..3edc8630eb4b 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -1411,6 +1411,7 @@ static struct inode *f2fs_alloc_inode(struct super_block *sb)
init_f2fs_rwsem(&fi->i_gc_rwsem[READ]);
init_f2fs_rwsem(&fi->i_gc_rwsem[WRITE]);
init_f2fs_rwsem(&fi->i_xattr_sem);
+ init_f2fs_rwsem(&fi->i_atomic_sem);

/* Will be used by directory only */
fi->i_dir_level = F2FS_SB(sb)->dir_level;
--
2.25.1