From: sunjunchao <sunjunchao@xxxxxxxxxxxxxx>
There is a BUG_ON statement which will be triggered in the
following scenario, let's remove it.
thread0 thread1
ext4_write_begin(inode0)
->ext4_try_to_write_inline_data()
written some bits successfully
ext4_write_end(inode0)
->ext4_write_inline_data_end()
ext4_write_begin(inode0)
->ext4_try_to_write_inline_data()
->ext4_convert_inline_data_to_extent()
->ext4_write_lock_xattr()
->ext4_destroy_inline_data_nolock()
->ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
->ext4_write_unlock_xattr()
->ext4_write_lock_xattr()
->BUG_ON(!ext4_has_inline_data()) will be triggered
The problematic logic is that ext4_write_end() test ext4_has_inline_data()
without holding xattr_sem, and ext4_write_inline_data_end() test it again using
a BUG_ON() with holding xattr_sem.
Signed-off-by: JunChao Sun <sunjunchao2870@xxxxxxxxx>
---
fs/ext4/inline.c | 6 +++++-
fs/ext4/inode.c | 5 ++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 1602d74..b5752975 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -753,7 +753,11 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
goto out;
}
ext4_write_lock_xattr(inode, &no_expand);
- BUG_ON(!ext4_has_inline_data(inode));
+ if (!ext4_has_inline_data(inode)) {
+ ext4_write_unlock_xattr(inode, &no_expand);
+ brelse(iloc.bh);
+ return -ENODATA;
+ }
/*
* ei->i_inline_off may have changed since
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index bf0b7de..7502657 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1317,7 +1317,10 @@ static int ext4_write_end(struct file *file,
if (ext4_has_inline_data(inode) &&
ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA))
- return ext4_write_inline_data_end(inode, pos, len, copied, page);
+ ret = ext4_write_inline_data_end(inode, pos, len, copied, page);
+
+ if (ret != -ENODATA)
+ return ret;
copied = block_write_end(file, mapping, pos, len, copied, page, fsdata);
/*
--
1.8.3.1
