Re: [PATCH] ocfs2: validate bg_bits during freefrag scan

[ZhengYuan Huang]

On Thu, Apr 9, 2026 at 2:50 PM Heming Zhao <heming.zhao@xxxxxxxx> wrote:
> > diff --git a/fs/ocfs2/ioctl.c b/fs/ocfs2/ioctl.c
> > index b6864602814c..a341a599e440 100644
> > --- a/fs/ocfs2/ioctl.c
> > +++ b/fs/ocfs2/ioctl.c
> > @@ -441,7 +441,7 @@ static int ocfs2_info_freefrag_scan_chain(struct ocfs2_super *osb,
> >       struct buffer_head *bh = NULL;
> >       struct ocfs2_group_desc *bg = NULL;
> >
> > -     unsigned int max_bits, num_clusters;
> > +     unsigned int max_bits, max_bitmap_bits, num_clusters;
> >       unsigned int offset = 0, cluster, chunk;
> >       unsigned int chunk_free, last_chunksize = 0;
> >
> > @@ -474,11 +474,27 @@ static int ocfs2_info_freefrag_scan_chain(struct ocfs2_super *osb,
> >               }
> >
> >               bg = (struct ocfs2_group_desc *)bh->b_data;
> > +             max_bits = le16_to_cpu(bg->bg_bits);
> > +             max_bitmap_bits = 8U * le16_to_cpu(bg->bg_size);
>
> The bg->bg_size can also change.
> It is better to use ocfs2_group_bitmap_size(osb->sb, 1, osb->s_feature_incompat)*8
> to retrieve the max_bitmap_bits value.
>
> > +
> > +             /*
> > +              * Non-coherent scans read raw blocks and do not get the
> > +              * bg_bits <= bg_size * 8 validation from
> > +              * ocfs2_read_group_descriptor().
> > +              */
> > +             if (max_bits > max_bitmap_bits) {
> > +                     mlog(ML_ERROR,
> > +                          "Group desc #%llu has %u bits but bitmap holds %u\n",
> > +                          (unsigned long long)blkno,
> > +                          max_bits,
> > +                          max_bitmap_bits);
> > +                     status = -EIO;
> > +                     goto bail;
>
> Since this function is used to report chain states, it's better to continue
> instead of breaking. In my view, reporting the error message and overwriting
> max_bits with max_bitmap_bits is sufficient.
>
> Thanks,
> Heming

Since this function is mainly used for reporting chain states, it makes sense
to continue instead of failing hard here.

I'll change it to log the error and clamp max_bits to max_bitmap_bits,
so we can still report as much information as possible.

Thanks,
ZhengYuan Huang