fix-race-while-writing-superblock

   1 jbd2: fix race when writing superblock
   2
   3 The jbd2 superblock is lockless now, so there is probably a race
   4 condition between writing it so disk and modifing contents of it, which
   5 may lead to checksum error. The following race is the one case that we
   6 have captured.
   7
   8 jbd2                                fsstress
   9 jbd2_journal_commit_transaction
  10  jbd2_journal_update_sb_log_tail
  11   jbd2_write_superblock
  12    jbd2_superblock_csum_set         jbd2_journal_revoke
  13                                      jbd2_journal_set_features(revork)
  14                                      modify superblock
  15    submit_bh(checksum incorrect)
  16
  17 Fix this by locking the buffer head before modifing it.  We always
  18 write the jbd2 superblock after we modify it, so this just means
  19 calling the lock_buffer() a little earlier.
  20
  21 This checksum corruption problem can be reproduced by xfstests
  22 generic/475.
  23
  24 Reported-by: zhangyi (F) <yi.zhang@huawei.com>
  25 Suggested-by: Jan Kara <jack@suse.cz>
  26 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  27 ---
  28  fs/jbd2/journal.c | 52 +++++++++++++++++++++++++++-------------------------
  29  1 file changed, 27 insertions(+), 25 deletions(-)
  30
  31 diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
  32 index 88d8f22d2cba..67ac91b53050 100644
  33 --- a/fs/jbd2/journal.c
  34 +++ b/fs/jbd2/journal.c
  35 @@ -1356,6 +1356,10 @@ static int journal_reset(journal_t *journal)
  36         return jbd2_journal_start_thread(journal);
  37  }
  38
  39 +/*
  40 + * This function expects that the caller will have locked the journal
  41 + * buffer head, and will return with it unlocked
  42 + */
  43  static int jbd2_write_superblock(journal_t *journal, int write_flags)
  44  {
  45         struct buffer_head *bh = journal->j_sb_buffer;
  46 @@ -1365,7 +1369,6 @@ static int jbd2_write_superblock(journal_t *journal, int write_flags)
  47         trace_jbd2_write_superblock(journal, write_flags);
  48         if (!(journal->j_flags & JBD2_BARRIER))
  49                 write_flags &= ~(REQ_FUA | REQ_PREFLUSH);
  50 -       lock_buffer(bh);
  51         if (buffer_write_io_error(bh)) {
  52                 /*
  53                  * Oh, dear.  A previous attempt to write the journal
  54 @@ -1424,6 +1427,7 @@ int jbd2_journal_update_sb_log_tail(journal_t *journal, tid_t tail_tid,
  55         jbd_debug(1, "JBD2: updating superblock (start %lu, seq %u)\n",
  56                   tail_block, tail_tid);
  57
  58 +       lock_buffer(journal->j_sb_buffer);
  59         sb->s_sequence = cpu_to_be32(tail_tid);
  60         sb->s_start    = cpu_to_be32(tail_block);
  61
  62 @@ -1454,18 +1458,17 @@ static void jbd2_mark_journal_empty(journal_t *journal, int write_op)
  63         journal_superblock_t *sb = journal->j_superblock;
  64
  65         BUG_ON(!mutex_is_locked(&journal->j_checkpoint_mutex));
  66 -       read_lock(&journal->j_state_lock);
  67 -       /* Is it already empty? */
  68 -       if (sb->s_start == 0) {
  69 -               read_unlock(&journal->j_state_lock);
  70 +       lock_buffer(journal->j_sb_buffer);
  71 +       if (sb->s_start == 0) {         /* Is it already empty? */
  72 +               unlock_buffer(journal->j_sb_buffer);
  73                 return;
  74         }
  75 +
  76         jbd_debug(1, "JBD2: Marking journal as empty (seq %d)\n",
  77                   journal->j_tail_sequence);
  78
  79         sb->s_sequence = cpu_to_be32(journal->j_tail_sequence);
  80         sb->s_start    = cpu_to_be32(0);
  81 -       read_unlock(&journal->j_state_lock);
  82
  83         jbd2_write_superblock(journal, write_op);
  84
  85 @@ -1488,9 +1491,8 @@ void jbd2_journal_update_sb_errno(journal_t *journal)
  86         journal_superblock_t *sb = journal->j_superblock;
  87         int errcode;
  88
  89 -       read_lock(&journal->j_state_lock);
  90 +       lock_buffer(journal->j_sb_buffer);
  91         errcode = journal->j_errno;
  92 -       read_unlock(&journal->j_state_lock);
  93         if (errcode == -ESHUTDOWN)
  94                 errcode = 0;
  95         jbd_debug(1, "JBD2: updating superblock error (errno %d)\n", errcode);
  96 @@ -1894,28 +1896,27 @@ int jbd2_journal_set_features (journal_t *journal, unsigned long compat,
  97
  98         sb = journal->j_superblock;
  99
 100 +       /* Load the checksum driver if necessary */
 101 +       if ((journal->j_chksum_driver == NULL) &&
 102 +           INCOMPAT_FEATURE_ON(JBD2_FEATURE_INCOMPAT_CSUM_V3)) {
 103 +               journal->j_chksum_driver = crypto_alloc_shash("crc32c", 0, 0);
 104 +               if (IS_ERR(journal->j_chksum_driver)) {
 105 +                       printk(KERN_ERR "JBD2: Cannot load crc32c driver.\n");
 106 +                       journal->j_chksum_driver = NULL;
 107 +                       return 0;
 108 +               }
 109 +               /* Precompute checksum seed for all metadata */
 110 +               journal->j_csum_seed = jbd2_chksum(journal, ~0, sb->s_uuid,
 111 +                                                  sizeof(sb->s_uuid));
 112 +       }
 113 +
 114 +       lock_buffer(journal->j_sb_buffer);
 115 +
 116         /* If enabling v3 checksums, update superblock */
 117         if (INCOMPAT_FEATURE_ON(JBD2_FEATURE_INCOMPAT_CSUM_V3)) {
 118                 sb->s_checksum_type = JBD2_CRC32C_CHKSUM;
 119                 sb->s_feature_compat &=
 120                         ~cpu_to_be32(JBD2_FEATURE_COMPAT_CHECKSUM);
 121 -
 122 -               /* Load the checksum driver */
 123 -               if (journal->j_chksum_driver == NULL) {
 124 -                       journal->j_chksum_driver = crypto_alloc_shash("crc32c",
 125 -                                                                     0, 0);
 126 -                       if (IS_ERR(journal->j_chksum_driver)) {
 127 -                               printk(KERN_ERR "JBD2: Cannot load crc32c "
 128 -                                      "driver.\n");
 129 -                               journal->j_chksum_driver = NULL;
 130 -                               return 0;
 131 -                       }
 132 -
 133 -                       /* Precompute checksum seed for all metadata */
 134 -                       journal->j_csum_seed = jbd2_chksum(journal, ~0,
 135 -                                                          sb->s_uuid,
 136 -                                                          sizeof(sb->s_uuid));
 137 -               }
 138         }
 139
 140         /* If enabling v1 checksums, downgrade superblock */
 141 @@ -1927,6 +1928,7 @@ int jbd2_journal_set_features (journal_t *journal, unsigned long compat,
 142         sb->s_feature_compat    |= cpu_to_be32(compat);
 143         sb->s_feature_ro_compat |= cpu_to_be32(ro);
 144         sb->s_feature_incompat  |= cpu_to_be32(incompat);
 145 +       unlock_buffer(journal->j_sb_buffer);
 146
 147         return 1;
 148  #undef COMPAT_FEATURE_ON