Re: [PATCH] fs: ocfs2: fix kernel BUG in ocfs2_find_victim_chain

[Prithvi Tambewagh]

On Mon, Dec 01, 2025 at 03:07:56PM +0800, Joseph Qi wrote:
> 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > > Closes: https://syzkaller.appspot.com/bug?extid=96d38c6e1655c1420a72
> > > > Tested-by: syzbot+96d38c6e1655c1420a72@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > > Cc: stable@xxxxxxxxxxxxxxx
> > > > Signed-off-by: Prithvi Tambewagh <activprithvi@xxxxxxxxx>
> > > > ---
> > > >  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.

I get it. We are trying to catch a state of disk corruption, so your 
suggestion

le16_to_cpu(cl->cl_next_free_rec) > le16_to_cpu(cl->cl_count)

fits best here. Thanks...I will make v2 for the patch.

Best Reards,
Prithvi

> Joseph