jbd2: fix ocfs2 corrupt when updating journal superblock fails

commit 6f6a6fda294506dfe0e3e0a253bb2d2923f28f0a upstream.

If updating journal superblock fails after journal data has been
flushed, the error is omitted and this will mislead the caller as a
normal case.  In ocfs2, the checkpoint will be treated successfully
and the other node can get the lock to update. Since the sb_start is
still pointing to the old log block, it will rewrite the journal data
during journal recovery by the other node. Thus the new updates will
be overwritten and ocfs2 corrupts.  So in above case we have to return
the error, and ocfs2_commit_cache will take care of the error and
prevent the other node to do update first.  And only after recovering
journal it can do the new updates.

The issue discussion mail can be found at:
https://oss.oracle.com/pipermail/ocfs2-devel/2015-June/010856.html
http://comments.gmane.org/gmane.comp.file-systems.ext4/48841

[ Fixed bug in patch which allowed a non-negative error return from
  jbd2_cleanup_journal_tail() to leak out of jbd2_fjournal_flush(); this
  was causing xfstests ext4/306 to fail. -- Ted ]

Reported-by: Yiwen Jiang <jiangyiwen@huawei.com>
Signed-off-by: Joseph Qi <joseph.qi@huawei.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Tested-by: Yiwen Jiang <jiangyiwen@huawei.com>
Cc: Junxiao Bi <junxiao.bi@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index dadfedbfd35660b5c9eedb4518010999b4fabafe..6bb52859cb866a12e804453c77d3f0a301fe327b 100644 (file)
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -440,7 +440,7 @@ int jbd2_cleanup_journal_tail(journal_t *journal)
        unsigned long   blocknr;
 
        if (is_journal_aborted(journal))
-               return 1;
+               return -EIO;
 
        if (!jbd2_journal_get_log_tail(journal, &first_tid, &blocknr))
                return 1;
@@ -457,8 +457,7 @@ int jbd2_cleanup_journal_tail(journal_t *journal)
        if (journal->j_flags & JBD2_BARRIER)
                blkdev_issue_flush(journal->j_fs_dev, GFP_NOFS, NULL);
 
-       __jbd2_update_log_tail(journal, first_tid, blocknr);
-       return 0;
+       return __jbd2_update_log_tail(journal, first_tid, blocknr);
 }
 
 

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index aaa1a3f33b0e951150203e4426c377af0d1605e2..3e7ef8874ffb17929d185cdd57ed7d3ddfd86e42 100644 (file)
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -869,9 +869,10 @@ int jbd2_journal_get_log_tail(journal_t *journal, tid_t *tid,
  *
  * Requires j_checkpoint_mutex
  */
-void __jbd2_update_log_tail(journal_t *journal, tid_t tid, unsigned long block)
+int __jbd2_update_log_tail(journal_t *journal, tid_t tid, unsigned long block)
 {
        unsigned long freed;
+       int ret;
 
        BUG_ON(!mutex_is_locked(&journal->j_checkpoint_mutex));
 
@@ -881,7 +882,10 @@ void __jbd2_update_log_tail(journal_t *journal, tid_t tid, unsigned long block)
         * space and if we lose sb update during power failure we'd replay
         * old transaction with possibly newly overwritten data.
         */
-       jbd2_journal_update_sb_log_tail(journal, tid, block, WRITE_FUA);
+       ret = jbd2_journal_update_sb_log_tail(journal, tid, block, WRITE_FUA);
+       if (ret)
+               goto out;
+
        write_lock(&journal->j_state_lock);
        freed = block - journal->j_tail;
        if (block < journal->j_tail)
@@ -897,6 +901,9 @@ void __jbd2_update_log_tail(journal_t *journal, tid_t tid, unsigned long block)
        journal->j_tail_sequence = tid;
        journal->j_tail = block;
        write_unlock(&journal->j_state_lock);
+
+out:
+       return ret;
 }
 
 /*
@@ -1315,7 +1322,7 @@ static int journal_reset(journal_t *journal)
        return jbd2_journal_start_thread(journal);
 }
 
-static void jbd2_write_superblock(journal_t *journal, int write_op)
+static int jbd2_write_superblock(journal_t *journal, int write_op)
 {
        struct buffer_head *bh = journal->j_sb_buffer;
        journal_superblock_t *sb = journal->j_superblock;
@@ -1354,7 +1361,10 @@ static void jbd2_write_superblock(journal_t *journal, int write_op)
                printk(KERN_ERR "JBD2: Error %d detected when updating "
                       "journal superblock for %s.\n", ret,
                       journal->j_devname);
+               jbd2_journal_abort(journal, ret);
        }
+
+       return ret;
 }
 
 /**
@@ -1367,10 +1377,11 @@ static void jbd2_write_superblock(journal_t *journal, int write_op)
  * Update a journal's superblock information about log tail and write it to
  * disk, waiting for the IO to complete.
  */
-void jbd2_journal_update_sb_log_tail(journal_t *journal, tid_t tail_tid,
+int jbd2_journal_update_sb_log_tail(journal_t *journal, tid_t tail_tid,
                                     unsigned long tail_block, int write_op)
 {
        journal_superblock_t *sb = journal->j_superblock;
+       int ret;
 
        BUG_ON(!mutex_is_locked(&journal->j_checkpoint_mutex));
        jbd_debug(1, "JBD2: updating superblock (start %lu, seq %u)\n",
@@ -1379,13 +1390,18 @@ void jbd2_journal_update_sb_log_tail(journal_t *journal, tid_t tail_tid,
        sb->s_sequence = cpu_to_be32(tail_tid);
        sb->s_start    = cpu_to_be32(tail_block);
 
-       jbd2_write_superblock(journal, write_op);
+       ret = jbd2_write_superblock(journal, write_op);
+       if (ret)
+               goto out;
 
        /* Log is no longer empty */
        write_lock(&journal->j_state_lock);
        WARN_ON(!sb->s_sequence);
        journal->j_flags &= ~JBD2_FLUSHED;
        write_unlock(&journal->j_state_lock);
+
+out:
+       return ret;
 }
 
 /**
@@ -1922,7 +1938,14 @@ int jbd2_journal_flush(journal_t *journal)
                return -EIO;
 
        mutex_lock(&journal->j_checkpoint_mutex);
-       jbd2_cleanup_journal_tail(journal);
+       if (!err) {
+               err = jbd2_cleanup_journal_tail(journal);
+               if (err < 0) {
+                       mutex_unlock(&journal->j_checkpoint_mutex);
+                       goto out;
+               }
+               err = 0;
+       }
 
        /* Finally, mark the journal as really needing no recovery.
         * This sets s_start==0 in the underlying superblock, which is
@@ -1938,7 +1961,8 @@ int jbd2_journal_flush(journal_t *journal)
        J_ASSERT(journal->j_head == journal->j_tail);
        J_ASSERT(journal->j_tail_sequence == journal->j_transaction_sequence);
        write_unlock(&journal->j_state_lock);
-       return 0;
+out:
+       return err;
 }
 
 /**

diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 6e051f472edb6db62f7e292b71c3bc5b2c5575f5..0c67c1f2a890bb9a7993b610770b7ef124f1af91 100644 (file)
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -997,7 +997,7 @@ extern struct journal_head * jbd2_journal_get_descriptor_buffer(journal_t *);
 int jbd2_journal_next_log_block(journal_t *, unsigned long long *);
 int jbd2_journal_get_log_tail(journal_t *journal, tid_t *tid,
                              unsigned long *block);
-void __jbd2_update_log_tail(journal_t *journal, tid_t tid, unsigned long block);
+int __jbd2_update_log_tail(journal_t *journal, tid_t tid, unsigned long block);
 void jbd2_update_log_tail(journal_t *journal, tid_t tid, unsigned long block);
 
 /* Commit management */
@@ -1116,7 +1116,7 @@ extern int           jbd2_journal_recover    (journal_t *journal);
 extern int        jbd2_journal_wipe       (journal_t *, int);
 extern int        jbd2_journal_skip_recovery   (journal_t *);
 extern void       jbd2_journal_update_sb_errno(journal_t *);
-extern void       jbd2_journal_update_sb_log_tail      (journal_t *, tid_t,
+extern int        jbd2_journal_update_sb_log_tail      (journal_t *, tid_t,
                                unsigned long, int);
 extern void       __jbd2_journal_abort_hard    (journal_t *);
 extern void       jbd2_journal_abort      (journal_t *, int);