[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <95804297-3a21-4024-8eb0-e75e8a3c4f87@linux.alibaba.com>
Date: Mon, 1 Dec 2025 15:07:56 +0800
From: Joseph Qi <joseph.qi@...ux.alibaba.com>
To: Prithvi Tambewagh <activprithvi@...il.com>
Cc: mark@...heh.com, jlbec@...lplan.org, ocfs2-devel@...ts.linux.dev,
linux-kernel@...r.kernel.org, linux-kernel-mentees@...ts.linux.dev,
skhan@...uxfoundation.org, david.hunter.linux@...il.com, khalid@...nel.org,
syzbot+96d38c6e1655c1420a72@...kaller.appspotmail.com
Subject: Re: [PATCH] fs: ocfs2: fix kernel BUG in ocfs2_find_victim_chain
On 2025/12/1 14:24, Prithvi Tambewagh wrote:
> On Mon, Dec 01, 2025 at 10:51:49AM +0800, Joseph Qi wrote:
>>
>>
>> On 2025/11/30 18:46, 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 is 0, triggring the
>>> BUG_ON(!cl->cl_next_free_rec) condition and panicking the kernel.
>>>
>>> To fix this, `cl_next_free_rec` is checked inside the caller of
>>> ocfs2_find_victim_chain() i.e. ocfs2_claim_suballoc_bits() and if it is
>>> equal to 0, ocfs2_error() is called, to log the corruption and force the
>>> filesystem into read-only mode, to prevent further damage.
>>>
>>> Reported-by: syzbot+96d38c6e1655c1420a72@...kaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?extid=96d38c6e1655c1420a72
>>> Tested-by: syzbot+96d38c6e1655c1420a72@...kaller.appspotmail.com
>>> Cc: stable@...r.kernel.org
>>> Signed-off-by: Prithvi Tambewagh <activprithvi@...il.com>
>>> ---
>>> fs/ocfs2/suballoc.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c
>>> index 6ac4dcd54588..84bb2d11c2aa 100644
>>> --- a/fs/ocfs2/suballoc.c
>>> +++ b/fs/ocfs2/suballoc.c
>>> @@ -1993,6 +1993,13 @@ static int ocfs2_claim_suballoc_bits(struct ocfs2_alloc_context *ac,
>>>
>>> cl = (struct ocfs2_chain_list *) &fe->id2.i_chain;
>>>
>>
>> This blank line can be eliminated.
>>
>>> + if (le16_to_cpu(cl->cl_next_free_rec) == 0) {
>>
>> Better to add the upper limit check as well. e.g.
>>
>> !le16_to_cpu(cl->cl_next_free_rec) ||
>> le16_to_cpu(cl->cl_next_free_rec) > le16_to_cpu(cl->cl_count)
>
> Hello Joseph,
>
> I went through the code in fs/ocfs2/suballoc.c, like this function
> static inline u16 ocfs2_find_smallest_chain(struct ocfs2_chain_list *cl)
> {
> u16 curr, best;
>
> best = curr = 0;
> while (curr < le16_to_cpu(cl->cl_count)) {
> if (le32_to_cpu(cl->cl_recs[best].c_total) >
> le32_to_cpu(cl->cl_recs[curr].c_total))
> best = curr;
> curr++;
> }
> return best;
> }
>
> and in function ocfs2_block_group_alloc() these lines
> if (le16_to_cpu(cl->cl_next_free_rec) < le16_to_cpu(cl->cl_count))
> le16_add_cpu(&cl->cl_next_free_rec, 1);
>
After this, cl_next_free_rec may equal to cl_count.
> and observed that according to the architecture of ocfs2, the chain list is in the form of 0-indexed array. In that case, the change you suggested for upper limit, could be re-written as
> le16_to_cpu(cl->cl_next_free_rec) >= le16_to_cpu(cl->cl_count)
>
> since value of cl->cl_next_free_rec greater than or equal to cl->cl_count will indicate that there are no available chains. Can you please review this?
>
Yes, it's full. But 'cl_next_free_rec == cl_count' is a designed behavior, see mkfs or fsck.
Joseph
Powered by blists - more mailing lists