[PATCH 1/2] ext4: Fix fsmap end of range reporting with bigalloc

[PATCH 1/2] ext4: Fix fsmap end of range reporting with bigalloc

[Ojaswin Mujoo]

With bigalloc enabled, the logic to report last extent has a bug since
we try to use cluster units instead of block units. This can cause an issue
where extra incorrect entries might be returned back to the user. This was
flagged by generic/365 with 64k bs and -O bigalloc.

** Details of issue **

The issue was noticed on 5G 64k blocksize FS with -O bigalloc which has
only 1 bg.

$ xfs_io -c "fsmap -d" /mnt/scratch

  0: 253:48 [0..127]: static fs metadata 128   /* sb */
  1: 253:48 [128..255]: special 102:1 128   /* gdt */
  3: 253:48 [256..383]: special 102:3 128   /* block bitmap */
  4: 253:48 [384..2303]: unknown 1920       /* flex bg empty space */
  5: 253:48 [2304..2431]: special 102:4 128   /* inode bitmap */
  6: 253:48 [2432..4351]: unknown 1920      /* flex bg empty space */
  7: 253:48 [4352..6911]: inodes 2560
  8: 253:48 [6912..538623]: unknown 531712
  9: 253:48 [538624..10485759]: free space 9947136

The issue can be seen with:

$ xfs_io -c "fsmap -d 0 3" /mnt/scratch

  0: 253:48 [0..127]: static fs metadata 128
  1: 253:48 [384..2047]: unknown 1664

Only the first entry was expected to be returned but we get 2. This is
because:

ext4_getfsmap_datadev()
  first_cluster, last_cluster = 0
  ...
  info->gfi_last = true;
  ext4_getfsmap_datadev_helper(sb, end_ag, last_cluster + 1, 0, info);
    fsb = C2B(1) = 16
    fslen = 0
    ...
    /* Merge in any relevant extents from the meta_list */
    list_for_each_entry_safe(p, tmp, &info->gfi_meta_list, fmr_list) {
      ...
      // since fsb = 16, considers all metadata which starts before 16 blockno
      iter 1: error = ext4_getfsmap_helper(sb, info, p);  // p = sb (0,1), nop
        info->gfi_next_fsblk = 1
      iter 2: error = ext4_getfsmap_helper(sb, info, p);  // p = gdt (1,2), nop
        info->gfi_next_fsblk = 2
      iter 3: error = ext4_getfsmap_helper(sb, info, p);  // p = blk bitmap (2,3), nop
        info->gfi_next_fsblk = 3
      iter 4: error = ext4_getfsmap_helper(sb, info, p);  // p = ino bitmap (18,19)
        if (rec_blk > info->gfi_next_fsblk) { // (18 > 3)
          // emits an extra entry ** BUG **
        }
    }

Fix this by directly calling ext4_getfsmap_datadev() with a dummy record
that has fmr_physical set to (end_fsb + 1) instead of last_cluster + 1. By
using the block instead of cluster we get the correct behavior.

Replacing ext4_getfsmap_datadev_helper() with ext4_getfsmap_helper() is
okay since the gfi_lastfree and metadata checks in
ext4_getfsmap_datadev_helper() are anyways redundant when we only want to
emit the last allocated block of the range, as we have already taken care
of emitting metadata and any last free blocks.

Reported-by: Disha Goel <disgoel@linux.ibm.com>
Fixes: 4a622e4d477b ("ext4: fix FS_IOC_GETFSMAP handling")
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 fs/ext4/fsmap.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/fsmap.c b/fs/ext4/fsmap.c
index 383c6edea6dd..9d63c39f6077 100644
--- a/fs/ext4/fsmap.c
+++ b/fs/ext4/fsmap.c
@@ -526,6 +526,7 @@ static int ext4_getfsmap_datadev(struct super_block *sb,
 	ext4_group_t end_ag;
 	ext4_grpblk_t first_cluster;
 	ext4_grpblk_t last_cluster;
+	struct ext4_fsmap irec;
 	int error = 0;
 
 	bofs = le32_to_cpu(sbi->s_es->s_first_data_block);
@@ -609,10 +610,18 @@ static int ext4_getfsmap_datadev(struct super_block *sb,
 			goto err;
 	}
 
-	/* Report any gaps at the end of the bg */
+	/*
+	 * The dummy record below will cause ext4_getfsmap_helper() to report
+	 * any allocated blocks at the end of the range.
+	 */
+	irec.fmr_device = 0;
+	irec.fmr_physical = end_fsb + 1;
+	irec.fmr_length = 0;
+	irec.fmr_owner = EXT4_FMR_OWN_FREE;
+	irec.fmr_flags = 0;
+
 	info->gfi_last = true;
-	error = ext4_getfsmap_datadev_helper(sb, end_ag, last_cluster + 1,
-					     0, info);
+	error = ext4_getfsmap_helper(sb, info, &irec);
 	if (error)
 		goto err;
 
-- 
2.49.0

[PATCH 2/2] ext4: Fix reserved gdt blocks handling in fsmap

[Ojaswin Mujoo]

In some cases like small FSes with no meta_bg and where the resize doesn't
need extra gdt blocks as it can fit in the current one,
s_reserved_gdt_blocks is set as 0, which causes fsmap to emit a 0 length
entry, which is incorrect.

  $ mkfs.ext4 -b 65536 -O bigalloc /dev/sda 5G
  $ mount /dev/sda /mnt/scratch
  $ xfs_io -c "fsmap -d" /mnt/scartch

        0: 253:48 [0..127]: static fs metadata 128
        1: 253:48 [128..255]: special 102:1 128
        2: 253:48 [256..255]: special 102:2 0     <---- 0 len entry
        3: 253:48 [256..383]: special 102:3 128

Fix this by adding a check for this case.

Fixes: 0c9ec4beecac ("ext4: support GETFSMAP ioctls")
Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>
---
 fs/ext4/fsmap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/ext4/fsmap.c b/fs/ext4/fsmap.c
index 9d63c39f6077..91185c40f755 100644
--- a/fs/ext4/fsmap.c
+++ b/fs/ext4/fsmap.c
@@ -393,6 +393,14 @@ static unsigned int ext4_getfsmap_find_sb(struct super_block *sb,
 	/* Reserved GDT blocks */
 	if (!ext4_has_feature_meta_bg(sb) || metagroup < first_meta_bg) {
 		len = le16_to_cpu(sbi->s_es->s_reserved_gdt_blocks);
+
+		/*
+		 * mkfs.ext4 can set s_reserved_gdt_blocks as 0 in some cases,
+		 * check for that.
+		 */
+		if (!len)
+			return 0;
+
 		error = ext4_getfsmap_fill(meta_list, fsb, len,
 					   EXT4_FMR_OWN_RESV_GDT);
 		if (error)
-- 
2.49.0

Re: [PATCH 1/2] ext4: Fix fsmap end of range reporting with bigalloc

[Darrick J. Wong]

On Tue, Aug 05, 2025 at 02:00:30PM +0530, Ojaswin Mujoo wrote:
> With bigalloc enabled, the logic to report last extent has a bug since
> we try to use cluster units instead of block units. This can cause an issue
> where extra incorrect entries might be returned back to the user. This was
> flagged by generic/365 with 64k bs and -O bigalloc.
> 
> ** Details of issue **
> 
> The issue was noticed on 5G 64k blocksize FS with -O bigalloc which has
> only 1 bg.
> 
> $ xfs_io -c "fsmap -d" /mnt/scratch
> 
>   0: 253:48 [0..127]: static fs metadata 128   /* sb */
>   1: 253:48 [128..255]: special 102:1 128   /* gdt */
>   3: 253:48 [256..383]: special 102:3 128   /* block bitmap */
>   4: 253:48 [384..2303]: unknown 1920       /* flex bg empty space */
>   5: 253:48 [2304..2431]: special 102:4 128   /* inode bitmap */
>   6: 253:48 [2432..4351]: unknown 1920      /* flex bg empty space */
>   7: 253:48 [4352..6911]: inodes 2560
>   8: 253:48 [6912..538623]: unknown 531712
>   9: 253:48 [538624..10485759]: free space 9947136
> 
> The issue can be seen with:
> 
> $ xfs_io -c "fsmap -d 0 3" /mnt/scratch
> 
>   0: 253:48 [0..127]: static fs metadata 128
>   1: 253:48 [384..2047]: unknown 1664
> 
> Only the first entry was expected to be returned but we get 2. This is
> because:
> 
> ext4_getfsmap_datadev()
>   first_cluster, last_cluster = 0
>   ...
>   info->gfi_last = true;
>   ext4_getfsmap_datadev_helper(sb, end_ag, last_cluster + 1, 0, info);
>     fsb = C2B(1) = 16
>     fslen = 0
>     ...
>     /* Merge in any relevant extents from the meta_list */
>     list_for_each_entry_safe(p, tmp, &info->gfi_meta_list, fmr_list) {
>       ...
>       // since fsb = 16, considers all metadata which starts before 16 blockno
>       iter 1: error = ext4_getfsmap_helper(sb, info, p);  // p = sb (0,1), nop
>         info->gfi_next_fsblk = 1
>       iter 2: error = ext4_getfsmap_helper(sb, info, p);  // p = gdt (1,2), nop
>         info->gfi_next_fsblk = 2
>       iter 3: error = ext4_getfsmap_helper(sb, info, p);  // p = blk bitmap (2,3), nop
>         info->gfi_next_fsblk = 3
>       iter 4: error = ext4_getfsmap_helper(sb, info, p);  // p = ino bitmap (18,19)
>         if (rec_blk > info->gfi_next_fsblk) { // (18 > 3)
>           // emits an extra entry ** BUG **
>         }
>     }
> 
> Fix this by directly calling ext4_getfsmap_datadev() with a dummy record
> that has fmr_physical set to (end_fsb + 1) instead of last_cluster + 1. By
> using the block instead of cluster we get the correct behavior.
> 
> Replacing ext4_getfsmap_datadev_helper() with ext4_getfsmap_helper() is
> okay since the gfi_lastfree and metadata checks in
> ext4_getfsmap_datadev_helper() are anyways redundant when we only want to
> emit the last allocated block of the range, as we have already taken care
> of emitting metadata and any last free blocks.
> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Fixes: 4a622e4d477b ("ext4: fix FS_IOC_GETFSMAP handling")
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

Looks fine to me
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/ext4/fsmap.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/fsmap.c b/fs/ext4/fsmap.c
> index 383c6edea6dd..9d63c39f6077 100644
> --- a/fs/ext4/fsmap.c
> +++ b/fs/ext4/fsmap.c
> @@ -526,6 +526,7 @@ static int ext4_getfsmap_datadev(struct super_block *sb,
>  	ext4_group_t end_ag;
>  	ext4_grpblk_t first_cluster;
>  	ext4_grpblk_t last_cluster;
> +	struct ext4_fsmap irec;
>  	int error = 0;
>  
>  	bofs = le32_to_cpu(sbi->s_es->s_first_data_block);
> @@ -609,10 +610,18 @@ static int ext4_getfsmap_datadev(struct super_block *sb,
>  			goto err;
>  	}
>  
> -	/* Report any gaps at the end of the bg */
> +	/*
> +	 * The dummy record below will cause ext4_getfsmap_helper() to report
> +	 * any allocated blocks at the end of the range.
> +	 */
> +	irec.fmr_device = 0;
> +	irec.fmr_physical = end_fsb + 1;
> +	irec.fmr_length = 0;
> +	irec.fmr_owner = EXT4_FMR_OWN_FREE;
> +	irec.fmr_flags = 0;
> +
>  	info->gfi_last = true;
> -	error = ext4_getfsmap_datadev_helper(sb, end_ag, last_cluster + 1,
> -					     0, info);
> +	error = ext4_getfsmap_helper(sb, info, &irec);
>  	if (error)
>  		goto err;
>  
> -- 
> 2.49.0
> 
> 

Re: [PATCH 2/2] ext4: Fix reserved gdt blocks handling in fsmap

[Darrick J. Wong]

On Tue, Aug 05, 2025 at 02:00:31PM +0530, Ojaswin Mujoo wrote:
> In some cases like small FSes with no meta_bg and where the resize doesn't
> need extra gdt blocks as it can fit in the current one,
> s_reserved_gdt_blocks is set as 0, which causes fsmap to emit a 0 length
> entry, which is incorrect.
> 
>   $ mkfs.ext4 -b 65536 -O bigalloc /dev/sda 5G
>   $ mount /dev/sda /mnt/scratch
>   $ xfs_io -c "fsmap -d" /mnt/scartch
> 
>         0: 253:48 [0..127]: static fs metadata 128
>         1: 253:48 [128..255]: special 102:1 128
>         2: 253:48 [256..255]: special 102:2 0     <---- 0 len entry
>         3: 253:48 [256..383]: special 102:3 128
> 
> Fix this by adding a check for this case.
> 
> Fixes: 0c9ec4beecac ("ext4: support GETFSMAP ioctls")
> Signed-off-by: Ojaswin Mujoo <ojaswin@linux.ibm.com>

I had no idea that this could be zero, so....
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/ext4/fsmap.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/ext4/fsmap.c b/fs/ext4/fsmap.c
> index 9d63c39f6077..91185c40f755 100644
> --- a/fs/ext4/fsmap.c
> +++ b/fs/ext4/fsmap.c
> @@ -393,6 +393,14 @@ static unsigned int ext4_getfsmap_find_sb(struct super_block *sb,
>  	/* Reserved GDT blocks */
>  	if (!ext4_has_feature_meta_bg(sb) || metagroup < first_meta_bg) {
>  		len = le16_to_cpu(sbi->s_es->s_reserved_gdt_blocks);
> +
> +		/*
> +		 * mkfs.ext4 can set s_reserved_gdt_blocks as 0 in some cases,
> +		 * check for that.
> +		 */
> +		if (!len)
> +			return 0;
> +
>  		error = ext4_getfsmap_fill(meta_list, fsb, len,
>  					   EXT4_FMR_OWN_RESV_GDT);
>  		if (error)
> -- 
> 2.49.0
> 
> 

Re: [PATCH 1/2] ext4: Fix fsmap end of range reporting with bigalloc

[Theodore Ts'o]

On Tue, 05 Aug 2025 14:00:30 +0530, Ojaswin Mujoo wrote:
> With bigalloc enabled, the logic to report last extent has a bug since
> we try to use cluster units instead of block units. This can cause an issue
> where extra incorrect entries might be returned back to the user. This was
> flagged by generic/365 with 64k bs and -O bigalloc.
> 
> ** Details of issue **
> 
> [...]

Applied, thanks!

[1/2] ext4: Fix fsmap end of range reporting with bigalloc
      commit: bae76c035bf0852844151e68098c9b7cd63ef238
[2/2] ext4: Fix reserved gdt blocks handling in fsmap
      commit: 3ffbdd1f1165f1b2d6a94d1b1aabef57120deaf7

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>