From: Baokun Li <libaokun1@huawei.com>
To: <linux-ext4@vger.kernel.org>
Cc: <tytso@mit.edu>, <adilger.kernel@dilger.ca>, <jack@suse.cz>,
<ritesh.list@gmail.com>, <linux-kernel@vger.kernel.org>,
<jun.nie@linaro.org>, <ebiggers@kernel.org>,
<yi.zhang@huawei.com>, <yangerkun@huawei.com>,
<yukuai3@huawei.com>, <libaokun1@huawei.com>,
<syzbot+a158d886ca08a3fecca4@syzkaller.appspotmail.com>,
<stable@vger.kernel.org>
Subject: [PATCH v2] ext4: fix race condition between buffer write and page_mkwrite
Date: Tue, 30 May 2023 21:44:05 +0800 [thread overview]
Message-ID: <20230530134405.322194-1-libaokun1@huawei.com> (raw)
Syzbot reported a BUG_ON:
==================================================================
EXT4-fs (loop0): mounted filesystem without journal. Quota mode: none.
EXT4-fs error (device loop0): ext4_mb_generate_buddy:1098: group 0, block
bitmap and bg descriptor inconsistent: 25 vs 150994969 free clusters
------------[ cut here ]------------
kernel BUG at fs/ext4/ext4_jbd2.c:53!
invalid opcode: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 494 Comm: syz-executor.0 6.1.0-rc7-syzkaller-ga4412fdd49dc #0
RIP: 0010:__ext4_journal_stop+0x1b3/0x1c0
[...]
Call Trace:
ext4_write_inline_data_end+0xa39/0xdf0
ext4_da_write_end+0x1e2/0x950
generic_perform_write+0x401/0x5f0
ext4_buffered_write_iter+0x35f/0x640
ext4_file_write_iter+0x198/0x1cd0
vfs_write+0x8b5/0xef0
[...]
==================================================================
The above BUG_ON is triggered by the following race:
cpu1 cpu2
________________________|________________________
ksys_write
vfs_write
new_sync_write
ext4_file_write_iter
ext4_buffered_write_iter
generic_perform_write
ext4_da_write_begin
do_fault
do_page_mkwrite
ext4_page_mkwrite
ext4_convert_inline_data
ext4_convert_inline_data_nolock
ext4_destroy_inline_data_nolock
//clear EXT4_STATE_MAY_INLINE_DATA
ext4_map_blocks --> return error
ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)
ext4_block_write_begin
ext4_restore_inline_data
// set EXT4_STATE_MAY_INLINE_DATA
ext4_da_write_end
ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)
ext4_write_inline_data_end
handle=NULL
ext4_journal_stop(handle)
__ext4_journal_stop
ext4_put_nojournal(handle)
ref_cnt = (unsigned long)handle
BUG_ON(ref_cnt == 0) ---> BUG_ON
The root cause of this problem is that the ext4_convert_inline_data() in
ext4_page_mkwrite() does not grab i_rwsem, so it may race with
ext4_buffered_write_iter() and cause the write_begin() and write_end()
functions to be inconsistent and trigger BUG_ON.
To solve the above issue, we can not add inode_lock directly to
ext4_page_mkwrite(), which would not only cause performance degradation but
also ABBA deadlock (see Link). Hence we move ext4_convert_inline_data() to
ext4_file_mmap(), and only when inline_data is enabled and mmap a writeable
file in shared mode, we hold the lock to convert, which avoids the above
problems.
Link: https://lore.kernel.org/r/20230530102804.6t7np7om6tczscuo@quack3/
Reported-by: Jun Nie <jun.nie@linaro.org>
Closes: https://lore.kernel.org/lkml/63903521.5040307@huawei.com/t/
Reported-by: syzbot+a158d886ca08a3fecca4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?id=899b37f20ce4072bcdfecfe1647b39602e956e36
Fixes: 7b4cc9787fe3 ("ext4: evict inline data when writing to memory map")
CC: stable@vger.kernel.org # 4.12+
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
fs/ext4/file.c | 24 +++++++++++++++++++++++-
fs/ext4/inode.c | 4 ----
2 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index d101b3b0c7da..9df82d72eb90 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -795,7 +795,8 @@ static const struct vm_operations_struct ext4_file_vm_ops = {
static int ext4_file_mmap(struct file *file, struct vm_area_struct *vma)
{
struct inode *inode = file->f_mapping->host;
- struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
+ struct super_block *sb = inode->i_sb;
+ struct ext4_sb_info *sbi = EXT4_SB(sb);
struct dax_device *dax_dev = sbi->s_daxdev;
if (unlikely(ext4_forced_shutdown(sbi)))
@@ -808,6 +809,27 @@ static int ext4_file_mmap(struct file *file, struct vm_area_struct *vma)
if (!daxdev_mapping_supported(vma, dax_dev))
return -EOPNOTSUPP;
+ /*
+ * Writing via mmap has no logic to handle inline data, so we
+ * need to call ext4_convert_inline_data() to convert the inode
+ * to normal format before doing so, otherwise a BUG_ON will be
+ * triggered in ext4_writepages() due to the
+ * EXT4_STATE_MAY_INLINE_DATA flag. Moreover, we need to grab
+ * i_rwsem during conversion, since clearing and setting the
+ * inline data flag may race with ext4_buffered_write_iter()
+ * to trigger a BUG_ON.
+ */
+ if (ext4_has_feature_inline_data(sb) &&
+ vma->vm_flags & VM_SHARED && vma->vm_flags & VM_MAYWRITE) {
+ int err;
+
+ inode_lock(inode);
+ err = ext4_convert_inline_data(inode);
+ inode_unlock(inode);
+ if (err)
+ return err;
+ }
+
file_accessed(file);
if (IS_DAX(file_inode(file))) {
vma->vm_ops = &ext4_dax_vm_ops;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index ce5f21b6c2b3..31844c4ec9fe 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -6043,10 +6043,6 @@ vm_fault_t ext4_page_mkwrite(struct vm_fault *vmf)
filemap_invalidate_lock_shared(mapping);
- err = ext4_convert_inline_data(inode);
- if (err)
- goto out_ret;
-
/*
* On data journalling we skip straight to the transaction handle:
* there's no delalloc; page truncated will be checked later; the
--
2.31.1
next reply other threads:[~2023-05-30 13:46 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-30 13:44 Baokun Li [this message]
2023-05-31 13:15 ` [PATCH v2] ext4: fix race condition between buffer write and page_mkwrite Jan Kara
2023-06-04 3:04 ` Theodore Ts'o
2023-06-04 21:08 ` Theodore Ts'o
2023-06-05 1:58 ` Matthew Wilcox
2023-06-05 9:16 ` Jan Kara
2023-06-05 12:21 ` Jan Kara
2023-06-05 14:55 ` Matthew Wilcox
2023-06-05 15:08 ` Jan Kara
2023-06-06 12:19 ` Baokun Li
2024-04-15 4:28 ` Baokun Li
2024-04-15 12:34 ` Jan Kara
2024-04-15 14:07 ` Baokun Li
2023-06-05 2:17 ` Baokun Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230530134405.322194-1-libaokun1@huawei.com \
--to=libaokun1@huawei.com \
--cc=adilger.kernel@dilger.ca \
--cc=ebiggers@kernel.org \
--cc=jack@suse.cz \
--cc=jun.nie@linaro.org \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ritesh.list@gmail.com \
--cc=stable@vger.kernel.org \
--cc=syzbot+a158d886ca08a3fecca4@syzkaller.appspotmail.com \
--cc=tytso@mit.edu \
--cc=yangerkun@huawei.com \
--cc=yi.zhang@huawei.com \
--cc=yukuai3@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).