ext4: free the new extent block when getting its buffer fails

ext4_ext_grow_indepth() allocates and accounts a metadata block before calling sb_getblk_gfp(). If the buffer allocation fails, the new block has not been linked into the extent tree, but the error path returns without freeing it. This leaks the block and leaves i_blocks larger than the number of blocks reachable from the inode.

Commit 8a83ac54940d ("ext4: call bdev_getblk() from sb_getblk_gfp()") removed the implicit __GFP_NOFAIL from this allocation, making this error path reachable.

Free the new metadata block before returning -ENOMEM. Use ext4_free_blocks() to undo both the allocation and inode accounting.

Tested on Linux 6.12.1 by injecting a NULL result at the caller's buffer acquisition boundary. Before the fix, seven failed extent tree growth operations left seven allocated but unreachable blocks and i_blocks of 40 instead of 32 on a filesystem with 4 KiB blocks. After the fix, the same failures leave i_blocks at 32, and e2fsck -fn passes after a normal unmount. Retrying the operation, truncating the file, and growing the tree without injection also succeed.

Fixes 8a83ac54940d ("ext4: call bdev_getblk() from sb_getblk_gfp()")

Cc [email protected]

Signed-off-by Alan <[email protected]>

---
 fs/ext4/extents.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

fs/ext4/extents.c +5 -1

index ad0d80e33f6090..7358df54bcd4ae 100644
@@ -1333,8 +1333,12 @@ static int ext4_ext_grow_indepth(handle_t *handle, struct inode *inode,
 		return err;
 
 	bh = sb_getblk_gfp(inode->i_sb, newblock, __GFP_MOVABLE | GFP_NOFS);
-	if (unlikely(!bh))
+	if (unlikely(!bh)) {
+		/* The new block has not been linked into the extent tree yet. */
+		ext4_free_blocks(handle, inode, NULL, newblock, 1,
+				 EXT4_FREE_BLOCKS_METADATA);
 		return -ENOMEM;
+	}
 	lock_buffer(bh);
 
 	err = ext4_journal_get_create_access(handle, inode->i_sb, bh,

base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e

Signature · 1 line

2.53.0

ext4_ext_grow_indepth() allocates and accounts a metadata block before getting its buffer, obtaining journal create access and dirtying the new extent block. If any of these operations fails, the new block has not been linked into the extent tree. However, the error paths return without freeing it, leaking the block and leaving i_blocks larger than the number of blocks reachable from the inode.

Commit 8a83ac54940d ("ext4: call bdev_getblk() from sb_getblk_gfp()") removed the implicit __GFP_NOFAIL from the buffer allocation, making the allocation failure path reachable. The two journal error paths already lacked cleanup before that commit and are fixed here as well.

Route all three failures through a common cleanup label to free the unpublished metadata block and undo its inode accounting. Keep the existing buffer unlock and release handling. Once the new block has been linked by ext4_idx_store_pblock(), an ext4_mark_inode_dirty() failure must not free it because the extent tree already references it.

Tested on Linux 6.12.1 by injecting NULL for buffer acquisition and -EIO for each of the two metadata helper calls at their caller boundaries. Each unfixed case leaves seven unreachable blocks and i_blocks of 40 instead of 32 on a filesystem with 4 KiB blocks. With the fix, all three cases retain i_blocks of 32, preserve the original data and pass e2fsck -fn after a normal unmount. Retry, truncate and uninjected growth also pass for each case.

Fixes 8a83ac54940d ("ext4: call bdev_getblk() from sb_getblk_gfp()")

Cc [email protected]

Signed-off-by Alan <[email protected]>

---
 fs/ext4/extents.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

fs/ext4/extents.c +12 -4

index ad0d80e33f6090..f0b2852f8a9342 100644
@@ -1333,15 +1333,17 @@ static int ext4_ext_grow_indepth(handle_t *handle, struct inode *inode,
 		return err;
 
 	bh = sb_getblk_gfp(inode->i_sb, newblock, __GFP_MOVABLE | GFP_NOFS);
-	if (unlikely(!bh))
-		return -ENOMEM;
+	if (unlikely(!bh)) {
+		err = -ENOMEM;
+		goto out_free;
+	}
 	lock_buffer(bh);
 
 	err = ext4_journal_get_create_access(handle, inode->i_sb, bh,
 					     EXT4_JTR_NONE);
 	if (err) {
 		unlock_buffer(bh);
-		goto out;
+		goto out_free;
 	}
 
 	ext_size = sizeof(EXT4_I(inode)->i_data);

@@ -1366,7 +1368,7 @@ static int ext4_ext_grow_indepth(handle_t *handle, struct inode *inode,
 
 	err = ext4_handle_dirty_metadata(handle, inode, bh);
 	if (err)
-		goto out;
+		goto out_free;
 
 	/* Update top-level index: num,max,pointer */
 	neh = ext_inode_hdr(inode);

@@ -1389,6 +1391,12 @@ static int ext4_ext_grow_indepth(handle_t *handle, struct inode *inode,
 	brelse(bh);
 
 	return err;
+
+out_free:
+	/* The new block has not been linked into the extent tree yet. */
+	ext4_free_blocks(handle, inode, NULL, newblock, 1,
+			 EXT4_FREE_BLOCKS_METADATA);
+	goto out;
 }
 
 /*

base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e

Signature · 1 line

2.53.0