[lvc-project] [RESEND PATCH v2] ocfs2: introduce chain list sanity check with ocfs2_check_chain_list()

[Heming Zhao]

On Tue, Oct 28, 2025 at 09:53:06AM +0800, Joseph Qi wrote:
> Hi,
> It seems that these checks can also been moved into ocfs2_validate_inode_block()?
> 
> Thanks,
> Joseph

The patch checks 'block group' not 'inode block', the signatures are
different: OCFS2_GROUP_DESC_SIGNATURE vs. OCFS2_INODE_SIGNATURE.
In my view, current code is correct.

- Heming

> 
> On 2025/10/27 20:56, Dmitry Antipov wrote:
> > Introduce 'ocfs2_check_chain_list()' to check whether a) the maximum
> > amount of chain records in 'struct ocfs2_chain_list' matches the value
> > calculated based on the filesystem block size, and b) the next free slot
> > index is within the valid range, and use the aforementioned function
> > in 'ocfs2_block_group_alloc()' and 'ocfs2_claim_suballoc_bits()'.
> > 
> > Reported-by: syzbot+77026564530dbc29b854 at syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=77026564530dbc29b854
> > Reported-by: syzbot+5054473a31f78f735416 at syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=5054473a31f78f735416
> > Signed-off-by: Dmitry Antipov <dmantipov at yandex.ru>
> > ---
> > note: resend to ensure that it's reviewed and hopefully good enough for
> >       mm-nonmm-unstable of -mm
> > v2: bail on cl->cl_next_free_rec > cl->cl_count (the name is misleading,
> >     cl_next_free_rec is a member count (and so can be equal to array
> >     size) rather than member index)
> > ---
> >  fs/ocfs2/suballoc.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c
> > index 6ac4dcd54588..9969a041ab18 100644
> > --- a/fs/ocfs2/suballoc.c
> > +++ b/fs/ocfs2/suballoc.c
> > @@ -649,6 +649,16 @@ ocfs2_block_group_alloc_discontig(handle_t *handle,
> >  	return status ? ERR_PTR(status) : bg_bh;
> >  }
> >  
> > +static int ocfs2_check_chain_list(struct ocfs2_chain_list *cl,
> > +				  struct super_block *sb)
> > +{
> > +	if (le16_to_cpu(cl->cl_count) != ocfs2_chain_recs_per_inode(sb))
> > +		return -EINVAL;
> > +	if (le16_to_cpu(cl->cl_next_free_rec) > le16_to_cpu(cl->cl_count))
> > +		return -EINVAL;
> > +	return 0;
> > +}
> > +
> >  /*
> >   * We expect the block group allocator to already be locked.
> >   */
> > @@ -671,6 +681,10 @@ static int ocfs2_block_group_alloc(struct ocfs2_super *osb,
> >  	BUG_ON(ocfs2_is_cluster_bitmap(alloc_inode));
> >  
> >  	cl = &fe->id2.i_chain;
> > +	status = ocfs2_check_chain_list(cl, alloc_inode->i_sb);
> > +	if (status)
> > +		goto bail;
> > +
> >  	status = ocfs2_reserve_clusters_with_limit(osb,
> >  						   le16_to_cpu(cl->cl_cpg),
> >  						   max_block, flags, &ac);
> > @@ -1992,6 +2006,9 @@ static int ocfs2_claim_suballoc_bits(struct ocfs2_alloc_context *ac,
> >  	}
> >  
> >  	cl = (struct ocfs2_chain_list *) &fe->id2.i_chain;
> > +	status = ocfs2_check_chain_list(cl, ac->ac_inode->i_sb);
> > +	if (status)
> > +		goto bail;
> >  
> >  	victim = ocfs2_find_victim_chain(cl);
> >  	ac->ac_chain = victim;
>