diff options
| author | Andrew Morton <akpm@osdl.org> | 2004-08-07 00:52:47 -0700 |
|---|---|---|
| committer | Linus Torvalds <torvalds@ppc970.osdl.org> | 2004-08-07 00:52:47 -0700 |
| commit | 9d294bd89e1e8491e5521baa812329de1bb9cbf6 (patch) | |
| tree | f7ae1e63f6cf3293a300bb0986355a639891893d /fs | |
| parent | aa524be4777d03386a4c5a35573dc464a1371948 (diff) | |
| download | history-9d294bd89e1e8491e5521baa812329de1bb9cbf6.tar.gz | |
[PATCH] jbd: journal_head unmapping race fix
Fix a race identified by Chris Mason <mason@suse.com>
journal_unmap_buffer -> __dispose_buffers has the j_list_lock and the
jbd_lock_bh_state held.
journal_get_write_access calls journal_put_journal_head, which takes
jbd_lock_bh_journal_head(bh) and doesn't seem to have any other locks held.
Since journal_unmap_buffers trusts the buffer_jbd bit to see if we need to
call __dispose_buffer, and nobody seems to test buffer_jbd after taking
jbd_lock_bh_journal_head. The kernel dereferences a null jh pointer in
__journal_remove_journal_head.
The patch fixes this by using journal_grab_journal_head() in
journal_unmap_buffer(). It ensures that we either grab and pin the
journal_head if the bh has one, or we bale out if the bh doesn't have a
journal_head.
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Diffstat (limited to 'fs')
| -rw-r--r-- | fs/jbd/transaction.c | 15 |
1 files changed, 8 insertions, 7 deletions
diff --git a/fs/jbd/transaction.c b/fs/jbd/transaction.c index 8f69595f9f0a24..18a678ce2591a9 100644 --- a/fs/jbd/transaction.c +++ b/fs/jbd/transaction.c @@ -1773,14 +1773,10 @@ static int journal_unmap_buffer(journal_t *journal, struct buffer_head *bh) jbd_lock_bh_state(bh); spin_lock(&journal->j_list_lock); - /* - * Now we have the locks, check again to see whether kjournald has - * taken the buffer off the transaction. - */ - if (!buffer_jbd(bh)) - goto zap_buffer; + jh = journal_grab_journal_head(bh); + if (!jh) + goto zap_buffer_no_jh; - jh = bh2jh(bh); transaction = jh->b_transaction; if (transaction == NULL) { /* First case: not on any transaction. If it @@ -1811,6 +1807,7 @@ static int journal_unmap_buffer(journal_t *journal, struct buffer_head *bh) spin_unlock(&journal->j_list_lock); jbd_unlock_bh_state(bh); spin_unlock(&journal->j_state_lock); + journal_put_journal_head(jh); return ret; } else { /* There is no currently-running transaction. So the @@ -1824,6 +1821,7 @@ static int journal_unmap_buffer(journal_t *journal, struct buffer_head *bh) spin_unlock(&journal->j_list_lock); jbd_unlock_bh_state(bh); spin_unlock(&journal->j_state_lock); + journal_put_journal_head(jh); return ret; } else { /* The orphan record's transaction has @@ -1847,6 +1845,7 @@ static int journal_unmap_buffer(journal_t *journal, struct buffer_head *bh) spin_unlock(&journal->j_list_lock); jbd_unlock_bh_state(bh); spin_unlock(&journal->j_state_lock); + journal_put_journal_head(jh); return 0; } else { /* Good, the buffer belongs to the running transaction. @@ -1860,6 +1859,8 @@ static int journal_unmap_buffer(journal_t *journal, struct buffer_head *bh) } zap_buffer: + journal_put_journal_head(jh); +zap_buffer_no_jh: spin_unlock(&journal->j_list_lock); jbd_unlock_bh_state(bh); spin_unlock(&journal->j_state_lock); |