diff mbox series

[v4] ocfs2: fix null-ptr-deref when journal load failed.

Message ID 20240902030844.422725-1-sunjunchao2870@gmail.com (mailing list archive)
State New
Headers show
Series [v4] ocfs2: fix null-ptr-deref when journal load failed. | expand

Commit Message

Julian Sun Sept. 2, 2024, 3:08 a.m. UTC
During the mounting process, if journal_reset() fails
because of too short journal, then lead to
jbd2_journal_load() fails with NULL j_sb_buffer.
Subsequently, ocfs2_journal_shutdown() calls
jbd2_journal_flush()->jbd2_cleanup_journal_tail()->
__jbd2_update_log_tail()->jbd2_journal_update_sb_log_tail()
->lock_buffer(journal->j_sb_buffer), resulting in a
null-pointer dereference error.

To resolve this issue, we should check the JBD2_LOADED flag to
ensure the journal was properly loaded. Additionally, use journal
instead of osb->journal directly to simplify the code.

Link: https://syzkaller.appspot.com/bug?extid=05b9b39d8bdfe1a0861f
Reported-by: syzbot+05b9b39d8bdfe1a0861f@syzkaller.appspotmail.com
Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>
---
 fs/ocfs2/journal.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Joseph Qi Sept. 2, 2024, 6:31 a.m. UTC | #1
On 9/2/24 11:08 AM, Julian Sun wrote:
> During the mounting process, if journal_reset() fails
> because of too short journal, then lead to
> jbd2_journal_load() fails with NULL j_sb_buffer.
> Subsequently, ocfs2_journal_shutdown() calls
> jbd2_journal_flush()->jbd2_cleanup_journal_tail()->
> __jbd2_update_log_tail()->jbd2_journal_update_sb_log_tail()
> ->lock_buffer(journal->j_sb_buffer), resulting in a
> null-pointer dereference error.
> 
> To resolve this issue, we should check the JBD2_LOADED flag to
> ensure the journal was properly loaded. Additionally, use journal
> instead of osb->journal directly to simplify the code.
> 
> Link: https://syzkaller.appspot.com/bug?extid=05b9b39d8bdfe1a0861f
> Reported-by: syzbot+05b9b39d8bdfe1a0861f@syzkaller.appspotmail.com
> Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> Signed-off-by: Julian Sun <sunjunchao2870@gmail.com>

Fixes: f6f50e28f0cb ("jbd2: Fail to load a journal if it is too short")
Cc: stable@vger.kernel.org
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> ---
>  fs/ocfs2/journal.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index 530fba34f6d3..1bf188b6866a 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -1055,7 +1055,7 @@ void ocfs2_journal_shutdown(struct ocfs2_super *osb)
>  	if (!igrab(inode))
>  		BUG();
>  
> -	num_running_trans = atomic_read(&(osb->journal->j_num_trans));
> +	num_running_trans = atomic_read(&(journal->j_num_trans));
>  	trace_ocfs2_journal_shutdown(num_running_trans);
>  
>  	/* Do a commit_cache here. It will flush our journal, *and*
> @@ -1074,9 +1074,10 @@ void ocfs2_journal_shutdown(struct ocfs2_super *osb)
>  		osb->commit_task = NULL;
>  	}
>  
> -	BUG_ON(atomic_read(&(osb->journal->j_num_trans)) != 0);
> +	BUG_ON(atomic_read(&(journal->j_num_trans)) != 0);
>  
> -	if (ocfs2_mount_local(osb)) {
> +	if (ocfs2_mount_local(osb) &&
> +	    (journal->j_journal->j_flags & JBD2_LOADED)) {
>  		jbd2_journal_lock_updates(journal->j_journal);
>  		status = jbd2_journal_flush(journal->j_journal, 0);
>  		jbd2_journal_unlock_updates(journal->j_journal);
diff mbox series

Patch

diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index 530fba34f6d3..1bf188b6866a 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -1055,7 +1055,7 @@  void ocfs2_journal_shutdown(struct ocfs2_super *osb)
 	if (!igrab(inode))
 		BUG();
 
-	num_running_trans = atomic_read(&(osb->journal->j_num_trans));
+	num_running_trans = atomic_read(&(journal->j_num_trans));
 	trace_ocfs2_journal_shutdown(num_running_trans);
 
 	/* Do a commit_cache here. It will flush our journal, *and*
@@ -1074,9 +1074,10 @@  void ocfs2_journal_shutdown(struct ocfs2_super *osb)
 		osb->commit_task = NULL;
 	}
 
-	BUG_ON(atomic_read(&(osb->journal->j_num_trans)) != 0);
+	BUG_ON(atomic_read(&(journal->j_num_trans)) != 0);
 
-	if (ocfs2_mount_local(osb)) {
+	if (ocfs2_mount_local(osb) &&
+	    (journal->j_journal->j_flags & JBD2_LOADED)) {
 		jbd2_journal_lock_updates(journal->j_journal);
 		status = jbd2_journal_flush(journal->j_journal, 0);
 		jbd2_journal_unlock_updates(journal->j_journal);