[PATCH kernel-4.19 1/3] jbd2: Drop unnecessary branch from jbd2_journal_forget()
From: Jan Kara <jack@suse.cz> mainline inclusion from mainline-5.5-rc1 commit 6d69843e5d3f0c394e1e3004cc2b36efbe402b71 category: bugfix bugzilla: 176007 CVE: NA --------------------------- We have cleared both dirty & jbddirty bits from the bh. So there's no difference between bforget() and brelse(). Thus there's no point jumping to no_jbd branch. Signed-off-by: Jan Kara <jack@suse.cz> Link: https://lore.kernel.org/r/20190809124233.13277-5-jack@suse.cz Signed-off-by: Theodore Ts'o <tytso@mit.edu> Conflicts: fs/jbd2/transaction.c Signed-off-by: yangerkun <yangerkun@huawei.com> Reviewed-by: Zhang Yi <yi.zhang@huawei.com> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- fs/jbd2/transaction.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index 4055929a043cf..c48658a4d53c0 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -1639,10 +1639,6 @@ int jbd2_journal_forget (handle_t *handle, struct buffer_head *bh) __jbd2_journal_file_buffer(jh, transaction, BJ_Forget); } else { __jbd2_journal_unfile_buffer(jh); - if (!buffer_jbd(bh)) { - spin_unlock(&journal->j_list_lock); - goto not_jbd; - } } spin_unlock(&journal->j_list_lock); } else if (jh->b_transaction) { -- 2.25.1
From: Jan Kara <jack@suse.cz> mainline inclusion from mainline-5.5-rc1 commit 2e710ff03fc4599059eeda68c8de2383e65af825 category: bugfix bugzilla: 176007 CVE: NA --------------------------- jbd2_journal_forget() jumps to 'not_jbd' branch which calls __bforget() in cases where the buffer is clean which is pointless. In case of failed assertion, it can be even argued that it is safer not to touch buffer's dirty bits. Also logically it makes more sense to just jump to 'drop' and that will make logic also simpler when we switch bh_state_lock to a spinlock. Signed-off-by: Jan Kara <jack@suse.cz> Link: https://lore.kernel.org/r/20190809124233.13277-6-jack@suse.cz Signed-off-by: Theodore Ts'o <tytso@mit.edu> Conflicts: fs/jbd2/transaction.c Signed-off-by: yangerkun <yangerkun@huawei.com> Reviewed-by: Zhang Yi <yi.zhang@huawei.com> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- fs/jbd2/transaction.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index c48658a4d53c0..3ac205eff7fb5 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -1591,7 +1591,7 @@ int jbd2_journal_forget (handle_t *handle, struct buffer_head *bh) if (!J_EXPECT_JH(jh, !jh->b_committed_data, "inconsistent data on disk")) { err = -EIO; - goto not_jbd; + goto drop; } /* keep track of whether or not this transaction modified us */ @@ -1680,7 +1680,7 @@ int jbd2_journal_forget (handle_t *handle, struct buffer_head *bh) if (!jh->b_cp_transaction) { JBUFFER_TRACE(jh, "belongs to none transaction"); spin_unlock(&journal->j_list_lock); - goto not_jbd; + goto drop; } /* @@ -1690,7 +1690,7 @@ int jbd2_journal_forget (handle_t *handle, struct buffer_head *bh) if (!buffer_dirty(bh)) { __jbd2_journal_remove_checkpoint(jh); spin_unlock(&journal->j_list_lock); - goto not_jbd; + goto drop; } /* @@ -1703,10 +1703,9 @@ int jbd2_journal_forget (handle_t *handle, struct buffer_head *bh) __jbd2_journal_file_buffer(jh, transaction, BJ_Forget); spin_unlock(&journal->j_list_lock); } - +drop: jbd_unlock_bh_state(bh); __brelse(bh); -drop: if (drop_reserve) { /* no need to reserve log space for this block -bzzz */ handle->h_total_credits++; -- 2.25.1
From: yangerkun <yangerkun@huawei.com> hulk inclusion category: bugfix bugzilla: 176007 CVE: NA --------------------------- jbd2_journal_put_journal_head protect jh with jbd_lock_bh_journal_head. jbd2_journal_forget protect jh with jbd_lock_bh_state. This two function can happen parallel, and this can lead the follow bug: [ 1140.658593] kasan: GPF could be caused by NULL-ptr deref or user memory access ... [ 1140.660011] general protection fault: 0000 [#1] SMP KASAN ... [ 1140.664723] RIP: 0010:__jbd2_journal_remove_checkpoint+0x7b/0x6a0 [ 1140.683008] Call Trace: [ 1140.683570] jbd2_journal_forget+0x564/0x840 [ 1140.684348] jbd2_journal_revoke+0x248/0x5b0 [ 1140.685101] __ext4_forget+0x341/0x5d0 [ 1140.685802] ext4_free_blocks+0x1233/0x1970 [ 1140.692235] ext4_ext_remove_space+0x1aaf/0x34b0 [ 1140.694614] ext4_ext_truncate+0x192/0x1e0 [ 1140.695320] ext4_truncate+0xad0/0x1020 [ 1140.698187] ext4_evict_inode+0xac6/0x15c0 [ 1140.700377] evict+0x2f6/0x650 [ 1140.701586] iput+0x3aa/0x740 [ 1140.702084] dentry_unlink_inode+0x2ff/0x3b0 [ 1140.702799] d_delete+0x1dd/0x240 [ 1140.703366] vfs_rmdir+0x2d5/0x430 [ 1140.703933] do_rmdir+0x2e1/0x380 [ 1140.705848] do_syscall_64+0xbd/0x3d0 [ 1140.707384] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Fix it by grab jh in jbd2_journal_forget, and put it at the end of jbd2_journal_forget. It is a part for 464170647b56 ("jbd2: Make state lock a spinlock"). Signed-off-by: yangerkun <yangerkun@huawei.com> Reviewed-by: Zhang Yi <yi.zhang@huawei.com> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- fs/jbd2/transaction.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index 3ac205eff7fb5..f56be34564e74 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -1580,11 +1580,13 @@ int jbd2_journal_forget (handle_t *handle, struct buffer_head *bh) BUFFER_TRACE(bh, "entry"); - jbd_lock_bh_state(bh); + jh = jbd2_journal_grab_journal_head(bh); + if (!jh) { + __bforget(bh); + return 0; + } - if (!buffer_jbd(bh)) - goto not_jbd; - jh = bh2jh(bh); + jbd_lock_bh_state(bh); /* Critical error: attempting to delete a bitmap buffer, maybe? * Don't do any jbd operations, and return an error. */ @@ -1704,18 +1706,14 @@ int jbd2_journal_forget (handle_t *handle, struct buffer_head *bh) spin_unlock(&journal->j_list_lock); } drop: - jbd_unlock_bh_state(bh); __brelse(bh); + jbd_unlock_bh_state(bh); + jbd2_journal_put_journal_head(jh); if (drop_reserve) { /* no need to reserve log space for this block -bzzz */ handle->h_total_credits++; } return err; - -not_jbd: - jbd_unlock_bh_state(bh); - __bforget(bh); - goto drop; } /** -- 2.25.1
participants (1)
-
Yang Yingliang