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

[Viacheslav Dubeyko]

On Tue, 2026-09-15 at 08:13 -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, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nilfs2/sufile.c b/fs/nilfs2/sufile.c
> index eceedca026..e3733d2be1 100644
> --- a/fs/nilfs2/sufile.c
> +++ b/fs/nilfs2/sufile.c
> @@ -864,15 +864,21 @@ ssize_t nilfs_sufile_get_suinfo(struct inode
> *sufile, __u64 segnum, void *buf,
>  	struct the_nilfs *nilfs = sufile->i_sb->s_fs_info;
>  	size_t offset;
>  	void *kaddr;
> -	unsigned long nsegs, segusages_per_block;
> +	unsigned long nsegments, nsegs, segusages_per_block;

The nsegments has the same meaning as nsegs. Why not reuse it? I think
we can do this. I think that introducing another local variable doesn't
make sense.

Thanks,
Slava. 

>  	ssize_t n;
>  	int ret, i, j;
>  
>  	down_read(&NILFS_MDT(sufile)->mi_sem);
>  
> +	nsegments = nilfs_sufile_get_nsegments(sufile);
> +	if (segnum >= nsegments) {
> +		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,
> +		      nsegments - segnum,
>  		      nsi);
>  	for (i = 0; i < nsegs; i += n, segnum += n) {
>  		n = min_t(unsigned long,