[PATCH v3] ocfs2: fix kernel BUG in ocfs2_find_victim_chain

Prithvi Tambewagh posted 1 patch 2 months, 1 week ago
fs/ocfs2/suballoc.c | 10 ++++++++++
1 file changed, 10 insertions(+)
[PATCH v3] ocfs2: fix kernel BUG in ocfs2_find_victim_chain
Posted by Prithvi Tambewagh 2 months, 1 week ago
syzbot reported a kernel BUG in ocfs2_find_victim_chain() because the
`cl_next_free_rec` field of the allocation chain list (next free slot in 
the chain list) is 0, triggring the BUG_ON(!cl->cl_next_free_rec) 
condition in ocfs2_find_victim_chain() and panicking the kernel.

To fix this, an if condition is introduced in ocfs2_claim_suballoc_bits(),
just before calling ocfs2_find_victim_chain(), the code block in it being 
executed when either of the following conditions is true:

1. `cl_next_free_rec` is equal to 0, indicating that there are no free 
chains in the allocation chain list
2. `cl_next_free_rec` is greater than `cl_count` (the total number of 
chains in the allocation chain list)

Either of them being true is indicative of the fact that there are no 
chains left for usage. 

This is addressed using ocfs2_error(), which prints
the error log for debugging purposes, rather than panicking the kernel.

Reported-by: syzbot+96d38c6e1655c1420a72@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=96d38c6e1655c1420a72
Tested-by: syzbot+96d38c6e1655c1420a72@syzkaller.appspotmail.com 
Cc: stable@vger.kernel.org
Signed-off-by: Prithvi Tambewagh <activprithvi@gmail.com>
---
v2->v3
 - Revise log message for reflecting changes from v1->v2
 - Format code style as suggested in v2

v1->v2:
 - Remove extra line before the if statement in patch
 - Add upper limit check for cl->cl_next_free_rec in the if condition

 fs/ocfs2/suballoc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c
index 6ac4dcd54588..e93fc842bb20 100644
--- a/fs/ocfs2/suballoc.c
+++ b/fs/ocfs2/suballoc.c
@@ -1992,6 +1992,16 @@ static int ocfs2_claim_suballoc_bits(struct ocfs2_alloc_context *ac,
 	}
 
 	cl = (struct ocfs2_chain_list *) &fe->id2.i_chain;
+	if (!le16_to_cpu(cl->cl_next_free_rec) ||
+	    le16_to_cpu(cl->cl_next_free_rec) > le16_to_cpu(cl->cl_count)) {
+		status = ocfs2_error(ac->ac_inode->i_sb,
+				     "Chain allocator dinode %llu has invalid next "
+				     "free chain record %u, but only %u total\n",
+				     (unsigned long long)le64_to_cpu(fe->i_blkno),
+				     le16_to_cpu(cl->cl_next_free_rec),
+				     le16_to_cpu(cl->cl_count));
+		goto bail;
+	}
 
 	victim = ocfs2_find_victim_chain(cl);
 	ac->ac_chain = victim;

base-commit: 939f15e640f193616691d3bcde0089760e75b0d3
-- 
2.34.1
Re: [PATCH v3] ocfs2: fix kernel BUG in ocfs2_find_victim_chain
Posted by Joseph Qi 2 months, 1 week ago

On 2025/12/1 21:07, Prithvi Tambewagh wrote:
> syzbot reported a kernel BUG in ocfs2_find_victim_chain() because the
> `cl_next_free_rec` field of the allocation chain list (next free slot in 
> the chain list) is 0, triggring the BUG_ON(!cl->cl_next_free_rec) 
> condition in ocfs2_find_victim_chain() and panicking the kernel.
> 
> To fix this, an if condition is introduced in ocfs2_claim_suballoc_bits(),
> just before calling ocfs2_find_victim_chain(), the code block in it being 
> executed when either of the following conditions is true:
> 
> 1. `cl_next_free_rec` is equal to 0, indicating that there are no free 
> chains in the allocation chain list
> 2. `cl_next_free_rec` is greater than `cl_count` (the total number of 
> chains in the allocation chain list)
> 
> Either of them being true is indicative of the fact that there are no 
> chains left for usage. 
> 
> This is addressed using ocfs2_error(), which prints
> the error log for debugging purposes, rather than panicking the kernel.
> 
> Reported-by: syzbot+96d38c6e1655c1420a72@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=96d38c6e1655c1420a72
> Tested-by: syzbot+96d38c6e1655c1420a72@syzkaller.appspotmail.com 
> Cc: stable@vger.kernel.org
> Signed-off-by: Prithvi Tambewagh <activprithvi@gmail.com>

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
> v2->v3
>  - Revise log message for reflecting changes from v1->v2
>  - Format code style as suggested in v2
> 
> v1->v2:
>  - Remove extra line before the if statement in patch
>  - Add upper limit check for cl->cl_next_free_rec in the if condition
> 
>  fs/ocfs2/suballoc.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c
> index 6ac4dcd54588..e93fc842bb20 100644
> --- a/fs/ocfs2/suballoc.c
> +++ b/fs/ocfs2/suballoc.c
> @@ -1992,6 +1992,16 @@ static int ocfs2_claim_suballoc_bits(struct ocfs2_alloc_context *ac,
>  	}
>  
>  	cl = (struct ocfs2_chain_list *) &fe->id2.i_chain;
> +	if (!le16_to_cpu(cl->cl_next_free_rec) ||
> +	    le16_to_cpu(cl->cl_next_free_rec) > le16_to_cpu(cl->cl_count)) {
> +		status = ocfs2_error(ac->ac_inode->i_sb,
> +				     "Chain allocator dinode %llu has invalid next "
> +				     "free chain record %u, but only %u total\n",
> +				     (unsigned long long)le64_to_cpu(fe->i_blkno),
> +				     le16_to_cpu(cl->cl_next_free_rec),
> +				     le16_to_cpu(cl->cl_count));
> +		goto bail;
> +	}
>  
>  	victim = ocfs2_find_victim_chain(cl);
>  	ac->ac_chain = victim;
> 
> base-commit: 939f15e640f193616691d3bcde0089760e75b0d3