On Sun, 2015-09-06 at 03:25 +0200, Bartosz Kwitniewski wrote:
> Commit a3ceb22921615827bfed39d7612a9a370bff0edb (upstream
> 79feb521a44705262d15cc819a4117a447b11ea7) in 3.2.x tree introduced
> __jbd2_update_log_tail which requires j_checkpoint_mutex, but locking of
> j_checkpoint_mutex in jbd2_journal_flush was not backported from upstream.
Oops.
> Fixes kernel BUG at fs/jbd2/journal.c:832 (__jbd2_update_log_tail):
> [] ? jbd2_cleanup_journal_tail+0x5d/0x61 [jbd2]
> [] ? jbd2_journal_flush+0xc2/0x156 [jbd2]
> [] ? ext4_freeze+0x2f/0x71 [ext4]
> [] ? filemap_write_and_wait+0x26/0x32
> [] ? freeze_super+0x8c/0xdd
> [] ? freeze_bdev+0x5b/0xa1
> [] ? start_cow_session+0xb3/0x2d6 [hcpdriver]
> [] ? printk+0x40/0x49
> [] ? alloc_cts_session+0x2e/0x33 [hcpdriver]
> [] ? ioctl_start_hcp_session+0x131/0x20d [hcpdriver]
> [] ? handle_ioctlStartHC2+0x95/0x1ab [hcpdriver]
> [] ? cow_ioctl_unlocked+0x13/0x18 [hcpdriver]
> [] ? do_vfs_ioctl+0x55a/0x5a9
> [] ? pax_randomize_kstack+0x4c/0x60
> [] ? sysret_check+0x20/0x62
> [] ? do_sys_open+0x11e/0x130
> [] ? sys_ioctl+0x3c/0x5f
> [] ? system_call_fastpath+0x16/0x1b
>
> Signed-off-by: Bartosz Kwitniewski <zerg2000@xxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> --- fs/jbd2/journal.c.orig> > 2015-08-12 16:33:24.000000000 +0200
> +++ fs/jbd2/journal.c> > 2015-09-06 00:57:56.890894891 +0200
> @@ -1828,10 +1828,13 @@ int jbd2_journal_flush(journal_t *journa
> > > if (is_journal_aborted(journal))
> > > > return -EIO;
>
> +> > mutex_lock(&journal->j_checkpoint_mutex);
> > > if (!err) {
> > > > err = jbd2_cleanup_journal_tail(journal);
> -> > > if (err < 0)
> +> > > if (err < 0) {
> +> > > > mutex_unlock(&journal->j_checkpoint_mutex);
> > > > > goto out;
> +> > > }
> > > > err = 0;
> > > }
>
> @@ -1841,6 +1844,7 @@ int jbd2_journal_flush(journal_t *journa
> > > * commits of data to the journal will restore the current
> > > * s_start value. */
> > > jbd2_mark_journal_empty(journal);
> +> > mutex_unlock(&journal->j_checkpoint_mutex);
> > > write_lock(&journal->j_state_lock);
> > > J_ASSERT(!journal->j_running_transaction);
> > > J_ASSERT(!journal->j_committing_transaction);
Why is it sufficient to add locking of j_checkpoint_mutex only in this
one function?
Shouldn't I cherry-pick commits 24bcc89c7e7c ("jbd2: split updating of
journal superblock and marking journal empty") and a78bb11d7acd ("jbd2:
protect all log tail updates with j_checkpoint_mutex") as well?
Ben.
--
Ben Hutchings
I say we take off; nuke the site from orbit. It's the only way to be sure.Attachment:
signature.asc
Description: This is a digitally signed message part