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-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230531115100.2779605-6-yi.zhang@huaweicloud.com>
Date:   Wed, 31 May 2023 19:51:00 +0800
From:   Zhang Yi <yi.zhang@...weicloud.com>
To:     linux-ext4@...r.kernel.org
Cc:     tytso@....edu, adilger.kernel@...ger.ca, jack@...e.cz,
        yi.zhang@...wei.com, yi.zhang@...weicloud.com, yukuai3@...wei.com,
        chengzhihao1@...wei.com
Subject: [PATCH 5/5] jbd2: fix a race when checking checkpoint buffer busy

From: Zhang Yi <yi.zhang@...wei.com>

Before removing checkpoint buffer from the t_checkpoint_list, we have to
check both BH_Dirty and BH_Lock bits together to distinguish buffers
have not been or were being written back. But __cp_buffer_busy() checks
them separately, it first check lock state and then check dirty, the
window between these two checks could be raced by writing back
procedure, which locks buffer and clears buffer dirty before I/O
completes. So it cannot guarantee checkpointing buffers been written
back to disk if some error happens later. Finally, it may clean
checkpoint transactions and lead to inconsistent filesystem.
jbd2_journal_forget() and __journal_try_to_free_buffer() also have the
same problem, so fix them by introduce a new helper to check the busy
state atomically.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217490
Cc: stable@...r.kernel.org
Signed-off-by: Zhang Yi <yi.zhang@...wei.com>
---
 fs/jbd2/checkpoint.c  | 8 ++++----
 fs/jbd2/transaction.c | 4 ++--
 include/linux/jbd2.h  | 3 +++
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 620f3d345f3d..2dde5fd1f0dd 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -45,11 +45,11 @@ static inline void __buffer_unlink(struct journal_head *jh)
  *
  * Requires j_list_lock
  */
-static inline bool __cp_buffer_busy(struct journal_head *jh)
+static inline bool cp_buffer_busy(struct journal_head *jh)
 {
 	struct buffer_head *bh = jh2bh(jh);
 
-	return (jh->b_transaction || buffer_locked(bh) || buffer_dirty(bh));
+	return (jh->b_transaction || __cp_buffer_busy(bh));
 }
 
 /*
@@ -369,7 +369,7 @@ static int journal_clean_one_cp_list(struct journal_head *jh, bool destroy)
 		jh = next_jh;
 		next_jh = jh->b_cpnext;
 
-		if (!destroy && __cp_buffer_busy(jh))
+		if (!destroy && cp_buffer_busy(jh))
 			return 0;
 
 		if (__jbd2_journal_remove_checkpoint(jh))
@@ -413,7 +413,7 @@ static unsigned long journal_shrink_one_cp_list(struct journal_head *jh,
 		next_jh = jh->b_cpnext;
 
 		(*nr_to_scan)--;
-		if (__cp_buffer_busy(jh))
+		if (cp_buffer_busy(jh))
 			continue;
 
 		nr_freed++;
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 18611241f451..04863787c93e 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -1784,7 +1784,7 @@ int jbd2_journal_forget(handle_t *handle, struct buffer_head *bh)
 		 * Otherwise, if the buffer has been written to disk,
 		 * it is safe to remove the checkpoint and drop it.
 		 */
-		if (!buffer_dirty(bh)) {
+		if (!__cp_buffer_busy(bh)) {
 			__jbd2_journal_remove_checkpoint(jh);
 			spin_unlock(&journal->j_list_lock);
 			goto drop;
@@ -2112,7 +2112,7 @@ __journal_try_to_free_buffer(journal_t *journal, struct buffer_head *bh)
 
 	jh = bh2jh(bh);
 
-	if (buffer_locked(bh) || buffer_dirty(bh))
+	if (__cp_buffer_busy(bh))
 		goto out;
 
 	if (jh->b_next_transaction != NULL || jh->b_transaction != NULL)
diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 91a2cf4bc575..b17d1efab787 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1440,6 +1440,9 @@ void jbd2_update_log_tail(journal_t *journal, tid_t tid, unsigned long block);
 extern void jbd2_journal_commit_transaction(journal_t *);
 
 /* Checkpoint list management */
+#define __cp_buffer_busy(bh) \
+	((bh)->b_state & ((1ul << BH_Dirty) | (1ul << BH_Lock)))
+
 void __jbd2_journal_clean_checkpoint_list(journal_t *journal, bool destroy);
 unsigned long jbd2_journal_shrink_checkpoint_list(journal_t *journal, unsigned long *nr_to_scan);
 int __jbd2_journal_remove_checkpoint(struct journal_head *);
-- 
2.31.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ