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

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

[Aldo Ariel Panzardo]

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@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 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;
 	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,
-- 
2.43.0

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@vger.kernel.org
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
>  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,

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

[Aldo Ariel Panzardo]

Hi Slava,

Good catch, no need for a separate variable. I'll reuse nsegs in v2.

Aldo