On Tue 30-05-23 21:44:05, Baokun Li wrote:

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=899b37f20ce4072bcdfecfe1647b39602e956e3... 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

Nice. Feel free to add:

Reviewed-by: Jan Kara jack@suse.cz

Honza

---

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