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  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]
Date:   Tue, 20 Dec 2022 23:05:51 +0800
From:   Zhihao Cheng <>
To:     <>, <>
CC:     <>, <>,
        <>, <>,
        <>, <>,
Subject: [PATCH] [RFC] Fix data missing when reusing bh which is ready to be checkpointed

From: zhanchengbin <>

Following process will make data lost and could lead to a filesystem
corrupted problem:

1. jh(bh) is inserted into T1->t_checkpoint_list, bh is dirty, and
   jh->b_transaction = NULL
2. T1 is added into journal->j_checkpoint_transactions.
3. Get bh prepare to write while doing checkpoing:
           PA				    PB
   do_get_write_access             jbd2_log_do_checkpoint
     if (buffer_dirty(bh))
      clear_buffer_dirty(bh)   // clear buffer dirty
				    transaction =
				    jh = transaction->t_checkpoint_list
				    if (!buffer_dirty(bh))
				      // bh won't be flushed
    __jbd2_journal_file_buffer(jh, transaction, BJ_Reserved)
4. Aborting journal/Power-cut before writing latest bh on journal area.

In this way we get a corrupted filesystem with bh'data lost.

Fix it by wrapping clear_buffer_dirty(bh) and jh->b_transaction setting
into journal->j_list_lock, so that jbd2_log_do_checkpoint() will wait
until jh's new transaction fininshed even bh is currently not dirty.

Cc: <>
Signed-off-by: zhanchengbin <>
Signed-off-by: Baokun Li <>
Signed-off-by: Zhihao Cheng <>
 This is a quick fix, I need some suggestions about this patch, whether
 it will import new problems if this patch is applied.
 Yi suggests that the formal solution could be splitting
 journal->j_list_lock into two locks: one protects checkpoint list and
 the other one for other lists. Besides, jh->b_state_lock should be
 held while traversing transaction->t_checkpoint_list in

 fs/jbd2/transaction.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 6a404ac1c178..d22460001d6b 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -990,6 +990,7 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
  	start_lock = jiffies;
+	spin_lock(&journal->j_list_lock);
 	/* If it takes too long to lock the buffer, trace it */
 	time_lock = jbd2_time_diff(start_lock, jiffies);
@@ -1039,6 +1040,7 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
 	error = -EROFS;
 	if (is_handle_aborted(handle)) {
+		spin_unlock(&journal->j_list_lock);
 		goto out;
@@ -1049,8 +1051,10 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
 	 * b_next_transaction points to it
 	if (jh->b_transaction == transaction ||
-	    jh->b_next_transaction == transaction)
+	    jh->b_next_transaction == transaction) {
+		spin_unlock(&journal->j_list_lock);
 		goto done;
+	}
 	 * this is the first time this transaction is touching this buffer,
@@ -1073,11 +1077,11 @@ do_get_write_access(handle_t *handle, struct journal_head *jh,
 		 * Paired with barrier in jbd2_write_access_granted()
-		spin_lock(&journal->j_list_lock);
 		__jbd2_journal_file_buffer(jh, transaction, BJ_Reserved);
 		goto done;
+	spin_unlock(&journal->j_list_lock);
 	 * If there is already a copy-out version of this buffer, then we don't
 	 * need to make another one

Powered by blists - more mailing lists