Re: [PATCH v2] nilfs2: validate segment number in nilfs_sufile_get_suinfo()

[Ryusuke Konishi]

On Thu, Sep 17, 2026 at 4:06 AM Viacheslav Dubeyko wrote:
>
> On Tue, 2026-09-15 at 16:52 -0300, Aldo Ariel Panzardo wrote:
> > nilfs_sufile_get_suinfo() subtracts the caller-provided segment
> > number
> > from the total number of segments without first checking its range.
> > If
> > the requested number is greater than the total, the unsigned
> > subtraction
> > wraps and the function may process segment numbers outside the
> > filesystem.
> >
> > Cache the total while holding the metadata semaphore and return no
> > entries
> > when the starting segment number is at or beyond the end.
> >
> > Fixes: 6c98cd4ecb0a ("nilfs2: segment usage file")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@xxxxxxxxx>
> > ---
> >  fs/nilfs2/sufile.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/nilfs2/sufile.c b/fs/nilfs2/sufile.c
> > index eceedca026..573646c5f7 100644
> > --- a/fs/nilfs2/sufile.c
> > +++ b/fs/nilfs2/sufile.c
> > @@ -870,10 +870,14 @@ ssize_t nilfs_sufile_get_suinfo(struct inode
> > *sufile, __u64 segnum, void *buf,
> >
> >       down_read(&NILFS_MDT(sufile)->mi_sem);
> >
> > +     nsegs = nilfs_sufile_get_nsegments(sufile);
> > +     if (segnum >= nsegs) {
> > +             ret = 0;
> > +             goto out;
> > +     }
> > +
> >       segusages_per_block =
> > nilfs_sufile_segment_usages_per_block(sufile);
> > -     nsegs = min_t(unsigned long,
> > -                   nilfs_sufile_get_nsegments(sufile) - segnum,
> > -                   nsi);
> > +     nsegs = min_t(unsigned long, nsegs - segnum, nsi);
> >       for (i = 0; i < nsegs; i += n, segnum += n) {
> >               n = min_t(unsigned long,
> >                         segusages_per_block -
>
> Looks good to me.
>
> Reviewed-by: Viacheslav Dubeyko <slava@xxxxxxxxxxx>
>
> Thanks,
> Slava.

Acked-by: Ryusuke Konishi <konishi.ryusuke@xxxxxxxxx>

Hi Viacheslav,

Please take this directly.

I also agree that this change makes sense, and my local test results
are as expected.

However, please drop the 'Cc: stable' tag since it does not cause any
fatal issues at the moment.
If a real-world problem is found or reported, I think it would be fine
to request a manual backport then.

While acquiring information on out-of-range segments due to
wrap-around is indeed possible, sufile is also a metadata management
file.
When accessing out-of-range areas, it simply hits as a hole block and
returns empty data.

Neither memory overrun in user space or kernel space, nor
out-of-bounds block device access or information leaks will occur.

Well-behaved userland tools read segment information based on the
statistics obtained via nilfs_get_sustat(), so no problem will occur
(the difference in behavior was visually indistinguishable).

Therefore, I modified the nilfs-utils to perform ill-behaved calls and
tested it.
Although out-of-range non-existent segments were displayed as empty
data, no critical issue that compromises kernel stability occurred.

At present, it does not appear to meet the basic criteria for stable
backporting, so when backporting, we need to be able to organize and
demonstrate the reasoning why it is necessary.

Thanks,
Ryusuke Konishi