| Message ID | 20220715125152.4022726-1-chengzhihao1@huawei.com |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
| Series | jbd2: Fix assertion 'jh->b_frozen_data == NULL' failure when journal aborted | expand |
On Fri, 15 Jul 2022 20:51:52 +0800, Zhihao Cheng wrote: > Following process will fail assertion 'jh->b_frozen_data == NULL' in > jbd2_journal_dirty_metadata(): > > jbd2_journal_commit_transaction > unlink(dir/a) > jh->b_transaction = trans1 > jh->b_jlist = BJ_Metadata > journal->j_running_transaction = NULL > trans1->t_state = T_COMMIT > unlink(dir/b) > handle->h_trans = trans2 > do_get_write_access > jh->b_modified = 0 > jh->b_frozen_data = frozen_buffer > jh->b_next_transaction = trans2 > jbd2_journal_dirty_metadata > is_handle_aborted > is_journal_aborted // return false > > [...] Applied, thanks! [1/1] jbd2: Fix assertion 'jh->b_frozen_data == NULL' failure when journal aborted commit: 4b18734448a3ee7100c5936a7fc0f9b1f2567e07 Best regards,
diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index e9c308ae475f..e0377f558eb1 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -1486,8 +1486,6 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh) struct journal_head *jh; int ret = 0; - if (is_handle_aborted(handle)) - return -EROFS; if (!buffer_jbd(bh)) return -EUCLEAN; @@ -1534,6 +1532,18 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh) journal = transaction->t_journal; spin_lock(&jh->b_state_lock); + if (is_handle_aborted(handle)) { + /* + * Check journal aborting with @jh->b_state_lock locked, + * since 'jh->b_transaction' could be replaced with + * 'jh->b_next_transaction' during old transaction + * committing if journal aborted, which may fail + * assertion on 'jh->b_frozen_data == NULL'. + */ + ret = -EROFS; + goto out_unlock_bh; + } + if (jh->b_modified == 0) { /* * This buffer's got modified and becoming part
Following process will fail assertion 'jh->b_frozen_data == NULL' in jbd2_journal_dirty_metadata(): jbd2_journal_commit_transaction unlink(dir/a) jh->b_transaction = trans1 jh->b_jlist = BJ_Metadata journal->j_running_transaction = NULL trans1->t_state = T_COMMIT unlink(dir/b) handle->h_trans = trans2 do_get_write_access jh->b_modified = 0 jh->b_frozen_data = frozen_buffer jh->b_next_transaction = trans2 jbd2_journal_dirty_metadata is_handle_aborted is_journal_aborted // return false --> jbd2 abort <-- while (commit_transaction->t_buffers) if (is_journal_aborted) jbd2_journal_refile_buffer __jbd2_journal_refile_buffer WRITE_ONCE(jh->b_transaction, jh->b_next_transaction) WRITE_ONCE(jh->b_next_transaction, NULL) __jbd2_journal_file_buffer(jh, BJ_Reserved) J_ASSERT_JH(jh, jh->b_frozen_data == NULL) // assertion failure ! The reproducer (See detail in [Link]) reports: ------------[ cut here ]------------ kernel BUG at fs/jbd2/transaction.c:1629! invalid opcode: 0000 [#1] PREEMPT SMP CPU: 2 PID: 584 Comm: unlink Tainted: G W 5.19.0-rc6-00115-g4a57a8400075-dirty #697 RIP: 0010:jbd2_journal_dirty_metadata+0x3c5/0x470 RSP: 0018:ffffc90000be7ce0 EFLAGS: 00010202 Call Trace: <TASK> __ext4_handle_dirty_metadata+0xa0/0x290 ext4_handle_dirty_dirblock+0x10c/0x1d0 ext4_delete_entry+0x104/0x200 __ext4_unlink+0x22b/0x360 ext4_unlink+0x275/0x390 vfs_unlink+0x20b/0x4c0 do_unlinkat+0x42f/0x4c0 __x64_sys_unlink+0x37/0x50 do_syscall_64+0x35/0x80 After journal aborting, __jbd2_journal_refile_buffer() is executed with holding @jh->b_state_lock, we can fix it by moving 'is_handle_aborted()' into the area protected by @jh->b_state_lock. Link: https://bugzilla.kernel.org/show_bug.cgi?id=216251 Fixes: 470decc613ab20 ("[PATCH] jbd2: initial copy of files from jbd") Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> --- fs/jbd2/transaction.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)