From: "Stephen C. Tweedie" Fix destruction of in-use journal_head journal_put_journal_head() can destroy a journal_head at any time as long as the jh's b_jcount is zero and b_transaction is NULL. It has no locking protection against the rest of the journaling code, as the lock it uses to protect b_jcount and bh->b_private is not used elsewhere in jbd. However, there are small windows where b_transaction is getting set temporarily to NULL during normal operations; typically this is happening in __journal_unfile_buffer(jh); __journal_file_buffer(jh, ...); call pairs, as __journal_unfile_buffer() will set b_transaction to NULL and __journal_file_buffer() re-sets it afterwards. A truncate running in parallel can lead to journal_unmap_buffer() destroying the jh if it occurs between these two calls. Fix this by adding a variant of __journal_unfile_buffer() which is only used for these temporary jh unlinks, and which leaves the b_transaction field intact so that we never leave a window open where b_transaction is NULL. Additionally, trap this error if it does occur, by checking against jh->b_jlist being non-null when we destroy a jh. Signed-off-by: Stephen Tweedie Signed-off-by: Andrew Morton --- 25-akpm/fs/jbd/commit.c | 2 +- 25-akpm/fs/jbd/journal.c | 1 + 25-akpm/fs/jbd/transaction.c | 27 +++++++++++++++++++-------- 25-akpm/include/linux/jbd.h | 1 + 4 files changed, 22 insertions(+), 9 deletions(-) diff -puN fs/jbd/commit.c~ext3-jbd-race-releasing-in-use-journal_heads fs/jbd/commit.c --- 25/fs/jbd/commit.c~ext3-jbd-race-releasing-in-use-journal_heads 2005-03-16 21:42:12.000000000 -0800 +++ 25-akpm/fs/jbd/commit.c 2005-03-16 21:42:12.000000000 -0800 @@ -341,7 +341,7 @@ write_out_data: BUFFER_TRACE(bh, "locked"); if (!inverted_lock(journal, bh)) goto write_out_data; - __journal_unfile_buffer(jh); + __journal_temp_unlink_buffer(jh); __journal_file_buffer(jh, commit_transaction, BJ_Locked); jbd_unlock_bh_state(bh); diff -puN fs/jbd/journal.c~ext3-jbd-race-releasing-in-use-journal_heads fs/jbd/journal.c --- 25/fs/jbd/journal.c~ext3-jbd-race-releasing-in-use-journal_heads 2005-03-16 21:42:12.000000000 -0800 +++ 25-akpm/fs/jbd/journal.c 2005-03-16 21:42:12.000000000 -0800 @@ -1803,6 +1803,7 @@ static void __journal_remove_journal_hea if (jh->b_transaction == NULL && jh->b_next_transaction == NULL && jh->b_cp_transaction == NULL) { + J_ASSERT_JH(jh, jh->b_jlist == BJ_None); J_ASSERT_BH(bh, buffer_jbd(bh)); J_ASSERT_BH(bh, jh2bh(jh) == bh); BUFFER_TRACE(bh, "remove journal_head"); diff -puN fs/jbd/transaction.c~ext3-jbd-race-releasing-in-use-journal_heads fs/jbd/transaction.c --- 25/fs/jbd/transaction.c~ext3-jbd-race-releasing-in-use-journal_heads 2005-03-16 21:42:12.000000000 -0800 +++ 25-akpm/fs/jbd/transaction.c 2005-03-16 21:42:12.000000000 -0800 @@ -1031,7 +1031,12 @@ int journal_dirty_data(handle_t *handle, /* journal_clean_data_list() may have got there first */ if (jh->b_transaction != NULL) { JBUFFER_TRACE(jh, "unfile from commit"); - __journal_unfile_buffer(jh); + __journal_temp_unlink_buffer(jh); + /* It still points to the committing + * transaction; move it to this one so + * that the refile assert checks are + * happy. */ + jh->b_transaction = handle->h_transaction; } /* The buffer will be refiled below */ @@ -1045,7 +1050,8 @@ int journal_dirty_data(handle_t *handle, if (jh->b_jlist != BJ_SyncData && jh->b_jlist != BJ_Locked) { JBUFFER_TRACE(jh, "not on correct data list: unfile"); J_ASSERT_JH(jh, jh->b_jlist != BJ_Shadow); - __journal_unfile_buffer(jh); + __journal_temp_unlink_buffer(jh); + jh->b_transaction = handle->h_transaction; JBUFFER_TRACE(jh, "file as data"); __journal_file_buffer(jh, handle->h_transaction, BJ_SyncData); @@ -1225,7 +1231,6 @@ int journal_forget (handle_t *handle, st JBUFFER_TRACE(jh, "belongs to current transaction: unfile"); - __journal_unfile_buffer(jh); drop_reserve = 1; /* @@ -1241,8 +1246,10 @@ int journal_forget (handle_t *handle, st */ if (jh->b_cp_transaction) { + __journal_temp_unlink_buffer(jh); __journal_file_buffer(jh, transaction, BJ_Forget); } else { + __journal_unfile_buffer(jh); journal_remove_journal_head(bh); __brelse(bh); if (!buffer_jbd(bh)) { @@ -1468,7 +1475,7 @@ __blist_del_buffer(struct journal_head * * * Called under j_list_lock. The journal may not be locked. */ -void __journal_unfile_buffer(struct journal_head *jh) +void __journal_temp_unlink_buffer(struct journal_head *jh) { struct journal_head **list = NULL; transaction_t *transaction; @@ -1485,7 +1492,7 @@ void __journal_unfile_buffer(struct jour switch (jh->b_jlist) { case BJ_None: - goto out; + return; case BJ_SyncData: list = &transaction->t_sync_datalist; break; @@ -1518,7 +1525,11 @@ void __journal_unfile_buffer(struct jour jh->b_jlist = BJ_None; if (test_clear_buffer_jbddirty(bh)) mark_buffer_dirty(bh); /* Expose it to the VM */ -out: +} + +void __journal_unfile_buffer(struct journal_head *jh) +{ + __journal_temp_unlink_buffer(jh); jh->b_transaction = NULL; } @@ -1928,7 +1939,7 @@ void __journal_file_buffer(struct journa } if (jh->b_transaction) - __journal_unfile_buffer(jh); + __journal_temp_unlink_buffer(jh); jh->b_transaction = transaction; switch (jlist) { @@ -2011,7 +2022,7 @@ void __journal_refile_buffer(struct jour */ was_dirty = test_clear_buffer_jbddirty(bh); - __journal_unfile_buffer(jh); + __journal_temp_unlink_buffer(jh); jh->b_transaction = jh->b_next_transaction; jh->b_next_transaction = NULL; __journal_file_buffer(jh, jh->b_transaction, BJ_Metadata); diff -puN include/linux/jbd.h~ext3-jbd-race-releasing-in-use-journal_heads include/linux/jbd.h --- 25/include/linux/jbd.h~ext3-jbd-race-releasing-in-use-journal_heads 2005-03-16 21:42:12.000000000 -0800 +++ 25-akpm/include/linux/jbd.h 2005-03-16 21:42:12.000000000 -0800 @@ -822,6 +822,7 @@ struct journal_s */ /* Filing buffers */ +extern void __journal_temp_unlink_buffer(struct journal_head *jh); extern void journal_unfile_buffer(journal_t *, struct journal_head *); extern void __journal_unfile_buffer(struct journal_head *); extern void __journal_refile_buffer(struct journal_head *); _