linux-ext4 - [PATCH] ext4: remove BUG_ON which will be triggered in race scenario

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230412074737.5769-1-sunjunchao2870@gmail.com>
Date:   Wed, 12 Apr 2023 00:47:37 -0700
From:   JunChao Sun <sunjunchao2870@...il.com>
To:     linux-ext4@...r.kernel.org
Cc:     tytso@....edu, yi.zhang@...wei.com, jack@...e.cz,
        sunjunchao <sunjunchao@...rongyun.com>,
        JunChao Sun <sunjunchao2870@...il.com>
Subject: [PATCH] ext4: remove BUG_ON which will be triggered in race scenario

From: sunjunchao <sunjunchao@...rongyun.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@...il.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