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

[Ryusuke Konishi]

On Fri, Sep 18, 2026 at 12:46 AM Ryusuke Konishi  wrote:
>
> 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

Hi Viacheslav,

To follow up on my previous email regarding the stable tag: since the
patch includes a Fixes tag, simply dropping 'Cc: stable' will not
prevent automatic pickup by AUTOSEL.

To strictly prevent automatic backporting while keeping it tracked,
please replace the tag with the following instead of dropping it
entirely:

Cc: stable+noautosel@xxxxxxxxxx # Non-fatal bug fix; defer backport
until a real issue is reported

Thanks,
Ryusuke Konishi