Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   JunChao Sun <sunjunchao2870@gmail.com>
To:     linux-ext4@vger.kernel.org
Cc:     tytso@mit.edu, yi.zhang@huawei.com, jack@suse.cz,
        sunjunchao <sunjunchao@yanrongyun.com>,
        JunChao Sun <sunjunchao2870@gmail.com>
Subject: [PATCH] ext4: remove BUG_ON which will be triggered in race scenario
Date:   Wed, 12 Apr 2023 00:47:37 -0700
Message-Id: <20230412074737.5769-1-sunjunchao2870@gmail.com>
Precedence: bulk

From: sunjunchao <sunjunchao@yanrongyun.com>

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@gmail.com>
---
 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