queue-4.19/jbd2-clear-dirty-flag-when-revoking-a-buffer-from-an-older-transaction.patch

   1 From 904cdbd41d749a476863a0ca41f6f396774f26e4 Mon Sep 17 00:00:00 2001
   2 From: "zhangyi (F)" <yi.zhang@huawei.com>
   3 Date: Sun, 10 Feb 2019 23:23:04 -0500
   4 Subject: jbd2: clear dirty flag when revoking a buffer from an older transaction
   5
   6 From: zhangyi (F) <yi.zhang@huawei.com>
   7
   8 commit 904cdbd41d749a476863a0ca41f6f396774f26e4 upstream.
   9
  10 Now, we capture a data corruption problem on ext4 while we're truncating
  11 an extent index block. Imaging that if we are revoking a buffer which
  12 has been journaled by the committing transaction, the buffer's jbddirty
  13 flag will not be cleared in jbd2_journal_forget(), so the commit code
  14 will set the buffer dirty flag again after refile the buffer.
  15
  16 fsx                               kjournald2
  17                                   jbd2_journal_commit_transaction
  18 jbd2_journal_revoke                commit phase 1~5...
  19  jbd2_journal_forget
  20    belongs to older transaction    commit phase 6
  21    jbddirty not clear               __jbd2_journal_refile_buffer
  22                                      __jbd2_journal_unfile_buffer
  23                                       test_clear_buffer_jbddirty
  24                                        mark_buffer_dirty
  25
  26 Finally, if the freed extent index block was allocated again as data
  27 block by some other files, it may corrupt the file data after writing
  28 cached pages later, such as during unmount time. (In general,
  29 clean_bdev_aliases() related helpers should be invoked after
  30 re-allocation to prevent the above corruption, but unfortunately we
  31 missed it when zeroout the head of extra extent blocks in
  32 ext4_ext_handle_unwritten_extents()).
  33
  34 This patch mark buffer as freed and set j_next_transaction to the new
  35 transaction when it already belongs to the committing transaction in
  36 jbd2_journal_forget(), so that commit code knows it should clear dirty
  37 bits when it is done with the buffer.
  38
  39 This problem can be reproduced by xfstests generic/455 easily with
  40 seeds (3246 3247 3248 3249).
  41
  42 Signed-off-by: zhangyi (F) <yi.zhang@huawei.com>
  43 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  44 Reviewed-by: Jan Kara <jack@suse.cz>
  45 Cc: stable@vger.kernel.org
  46 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  47
  48 ---
  49  fs/jbd2/transaction.c |   17 ++++++++++++-----
  50  1 file changed, 12 insertions(+), 5 deletions(-)
  51
  52 --- a/fs/jbd2/transaction.c
  53 +++ b/fs/jbd2/transaction.c
  54 @@ -1576,14 +1576,21 @@ int jbd2_journal_forget (handle_t *handl
  55                 /* However, if the buffer is still owned by a prior
  56                  * (committing) transaction, we can't drop it yet... */
  57                 JBUFFER_TRACE(jh, "belongs to older transaction");
  58 -               /* ... but we CAN drop it from the new transaction if we
  59 -                * have also modified it since the original commit. */
  60 +               /* ... but we CAN drop it from the new transaction through
  61 +                * marking the buffer as freed and set j_next_transaction to
  62 +                * the new transaction, so that not only the commit code
  63 +                * knows it should clear dirty bits when it is done with the
  64 +                * buffer, but also the buffer can be checkpointed only
  65 +                * after the new transaction commits. */
  66
  67 -               if (jh->b_next_transaction) {
  68 -                       J_ASSERT(jh->b_next_transaction == transaction);
  69 +               set_buffer_freed(bh);
  70 +
  71 +               if (!jh->b_next_transaction) {
  72                         spin_lock(&journal->j_list_lock);
  73 -                       jh->b_next_transaction = NULL;
  74 +                       jh->b_next_transaction = transaction;
  75                         spin_unlock(&journal->j_list_lock);
  76 +               } else {
  77 +                       J_ASSERT(jh->b_next_transaction == transaction);
  78
  79                         /*
  80                          * only drop a reference if this transaction modified