jbd2: fix race between write_metadata_buffer and get_write_access

The function jbd2_journal_write_metadata_buffer() calls
jbd_unlock_bh_state(bh_in) too early; this could potentially allow
another thread to call get_write_access on the buffer head, modify the
data, and dirty it, and allowing the wrong data to be written into the
journal.  Fortunately, if we lose this race, the only time this will
actually cause filesystem corruption is if there is a system crash or
other unclean shutdown of the system before the next commit can take
place.

Signed-off-by: dingdinghua <dingdinghua85@gmail.com>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
This commit is contained in:
dingdinghua 2009-07-13 17:55:35 -04:00 committed by Theodore Ts'o
parent 833576b362
commit 96577c4382
1 changed files with 11 additions and 9 deletions

View File

@ -297,6 +297,7 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction,
unsigned int new_offset; unsigned int new_offset;
struct buffer_head *bh_in = jh2bh(jh_in); struct buffer_head *bh_in = jh2bh(jh_in);
struct jbd2_buffer_trigger_type *triggers; struct jbd2_buffer_trigger_type *triggers;
journal_t *journal = transaction->t_journal;
/* /*
* The buffer really shouldn't be locked: only the current committing * The buffer really shouldn't be locked: only the current committing
@ -310,6 +311,11 @@ int jbd2_journal_write_metadata_buffer(transaction_t *transaction,
J_ASSERT_BH(bh_in, buffer_jbddirty(bh_in)); J_ASSERT_BH(bh_in, buffer_jbddirty(bh_in));
new_bh = alloc_buffer_head(GFP_NOFS|__GFP_NOFAIL); new_bh = alloc_buffer_head(GFP_NOFS|__GFP_NOFAIL);
/* keep subsequent assertions sane */
new_bh->b_state = 0;
init_buffer(new_bh, NULL, NULL);
atomic_set(&new_bh->b_count, 1);
new_jh = jbd2_journal_add_journal_head(new_bh); /* This sleeps */
/* /*
* If a new transaction has already done a buffer copy-out, then * If a new transaction has already done a buffer copy-out, then
@ -388,14 +394,6 @@ repeat:
kunmap_atomic(mapped_data, KM_USER0); kunmap_atomic(mapped_data, KM_USER0);
} }
/* keep subsequent assertions sane */
new_bh->b_state = 0;
init_buffer(new_bh, NULL, NULL);
atomic_set(&new_bh->b_count, 1);
jbd_unlock_bh_state(bh_in);
new_jh = jbd2_journal_add_journal_head(new_bh); /* This sleeps */
set_bh_page(new_bh, new_page, new_offset); set_bh_page(new_bh, new_page, new_offset);
new_jh->b_transaction = NULL; new_jh->b_transaction = NULL;
new_bh->b_size = jh2bh(jh_in)->b_size; new_bh->b_size = jh2bh(jh_in)->b_size;
@ -412,7 +410,11 @@ repeat:
* copying is moved to the transaction's shadow queue. * copying is moved to the transaction's shadow queue.
*/ */
JBUFFER_TRACE(jh_in, "file as BJ_Shadow"); JBUFFER_TRACE(jh_in, "file as BJ_Shadow");
jbd2_journal_file_buffer(jh_in, transaction, BJ_Shadow); spin_lock(&journal->j_list_lock);
__jbd2_journal_file_buffer(jh_in, transaction, BJ_Shadow);
spin_unlock(&journal->j_list_lock);
jbd_unlock_bh_state(bh_in);
JBUFFER_TRACE(new_jh, "file as BJ_IO"); JBUFFER_TRACE(new_jh, "file as BJ_IO");
jbd2_journal_file_buffer(new_jh, transaction, BJ_IO); jbd2_journal_file_buffer(new_jh, transaction, BJ_IO);