From: "Stephen C. Tweedie" <sct@redhat.com>
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 <sct@redhat.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---
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-09 19:30:21.000000000 -0800
+++ 25-akpm/fs/jbd/commit.c 2005-03-09 19:30:21.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-09 19:30:21.000000000 -0800
+++ 25-akpm/fs/jbd/journal.c 2005-03-09 19:57:26.000000000 -0800
@@ -1792,6 +1792,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-09 19:30:21.000000000 -0800
+++ 25-akpm/fs/jbd/transaction.c 2005-03-09 19:57:02.000000000 -0800
@@ -1030,7 +1030,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 */
@@ -1044,7 +1049,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);
@@ -1224,7 +1230,6 @@ int journal_forget (handle_t *handle, st
JBUFFER_TRACE(jh, "belongs to current transaction: unfile");
- __journal_unfile_buffer(jh);
drop_reserve = 1;
/*
@@ -1240,8 +1245,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)) {
@@ -1467,7 +1474,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;
@@ -1484,7 +1491,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;
@@ -1517,7 +1524,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;
}
@@ -1927,7 +1938,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) {
@@ -2010,7 +2021,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-09 19:30:45.000000000 -0800
+++ 25-akpm/include/linux/jbd.h 2005-03-09 19:57:46.000000000 -0800
@@ -817,6 +817,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 *);
_