public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: tytso@mit.edu
Cc: linux-ext4@vger.kernel.org, linux-ext4@vger.kernel.org
Subject: [PATCH 04/16] libext2fs: fix ind_punch recursive block computation
Date: Thu, 16 Oct 2025 08:40:49 -0700	[thread overview]
Message-ID: <176062915537.3343688.2245819523975318264.stgit@frogsfrogsfrogs> (raw)
In-Reply-To: <176062915393.3343688.9810444125172113159.stgit@frogsfrogsfrogs>

From: Darrick J. Wong <djwong@kernel.org>

generic/742 writes a large file and calls punch-alternating to punch out
every other block in the file.  Unfortunately, there's a bug in
ind_punch that causes it to turn a request to punch out file block 2060
into punching out every file block *starting* at block 2060.  The
emptying out of the block mapping causes a FIEMAP call in that test to
loop forever, which causes fuse4fs testing of ext3 to halt.

ext2fs_punch_ind recursively calls ind_punch on each level of the
indirect map to unmap file blocks.  The start and count parameters to
ind_punch are (I think) the file range to punch given a particular block
and level within the indirect map, but shifted down by the start of the
logical file range mapped by that level in the indirect tree.  To call
ind_punch for the next level down, we need to compute the new range.
But first, a diversion back to the failing testcase:

Note that file block 2060 is the first block mapped by the second
double-indirect block in the file:

(0-11):935-946, (IND):947, (12-1035):948-1971, (DIND):1972, (IND):1973,
(1036-2059):1974-2997, (IND):2998, (2060-3083):2999-4022, (IND):4023,
(3084-4107):4024-5047, (IND):5048, (4108-5131):5049-6072, (IND):6073,
(5132-6155):6074-7097, (IND):7098, (6156-7179):7099-8122, (IND):8123,
(7180-8203):8124-9147, (IND):9148, (8204-9227):9149-10172, (IND):10173,
(9228-9999):10174-10945 TOTAL: 10011

At this point, enabling PUNCH_DEBUG produces the following:

FUSE4FS (sda): tid=72529 fuse4fs_punch_range: ino=53 mode=0x3 offset=0x80c000 len=0x1000 start=0x80c end=0x80d
Main loop level 0, start 2060 count 1 max 12 num 12
Main loop level 1, start 2048 count 1 max 1024 num 1
Main loop level 2, start 1024 count 1 max 1048576 num 1
Entering ind_punch, level 2, start 1024, count 1, max 1
Reading indirect block 1972
start 1024 offset 0 start2 1024 count 1 count2 1
Entering ind_punch, level 1, start 1024, count 1, max 1024
Reading indirect block 2998
start 1024 offset 1024 start2 0 count 1 count2 1
Entering ind_punch, level 0, start 0, count 18446744073709550593, max 1024

This is wrong, because we want to punch *one* block, not some gigantic
number of blocks!  This huge value is actually -1023, which is the
result of the expression (count - offset).  Ooops, that's why we unmap
every block in this indirect block!

Note the suspicious 7th argument to the nested ind_punch call:

	count - offset

This doesn't smell right, because we're subtracting a position from a
length.  The end of the range for the next level down should be the
difference between the end and the start of the range in the current
level after accounting for our current position within that level.  In
other words, the smaller of end - start and end - offset.

Cc: <linux-ext4@vger.kernel.org> # v1.42
Fixes: 3adb9374fb9273 ("libext2fs: Add new function ext2fs_punch()")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 lib/ext2fs/punch.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)


diff --git a/lib/ext2fs/punch.c b/lib/ext2fs/punch.c
index 19b6a37824c589..c48e74ef799a82 100644
--- a/lib/ext2fs/punch.c
+++ b/lib/ext2fs/punch.c
@@ -54,6 +54,7 @@ static errcode_t ind_punch(ext2_filsys fs, struct ext2_inode *inode,
 	blk_t		b;
 	int		i;
 	blk64_t		offset, incr;
+	const blk64_t	end = start + count;
 	int		freed = 0;
 
 #ifdef PUNCH_DEBUG
@@ -62,13 +63,14 @@ static errcode_t ind_punch(ext2_filsys fs, struct ext2_inode *inode,
 #endif
 	incr = 1ULL << ((EXT2_BLOCK_SIZE_BITS(fs->super) - 2) * level);
 	for (i = 0, offset = 0; i < max; i++, p++, offset += incr) {
-		if (offset >= start + count)
+		if (offset >= end)
 			break;
 		if (*p == 0 || (offset+incr) <= start)
 			continue;
 		b = *p;
 		if (level > 0) {
 			blk_t start2;
+			blk_t count2;
 #ifdef PUNCH_DEBUG
 			printf("Reading indirect block %u\n", b);
 #endif
@@ -76,9 +78,15 @@ static errcode_t ind_punch(ext2_filsys fs, struct ext2_inode *inode,
 			if (retval)
 				return retval;
 			start2 = (start > offset) ? start - offset : 0;
+			count2 = end - ((start > offset) ? start : offset);
+#ifdef PUNCH_DEBUG
+			printf("start %llu offset %llu count %llu end %llu "
+			       "incr %llu start2 %u count2 %u\n", start,
+			       offset, count, end, incr, start2, count2);
+#endif
 			retval = ind_punch(fs, inode, block_buf + fs->blocksize,
 					   (blk_t *) block_buf, level - 1,
-					   start2, count - offset,
+					   start2, count2,
 					   fs->blocksize >> 2);
 			if (retval)
 				return retval;


  parent reply	other threads:[~2025-10-16 15:40 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-16 15:39 [PATCHSET] fuse2fs: round 6 bug fixes Darrick J. Wong
2025-10-16 15:40 ` [PATCH 01/16] debian/rules: remove extra pkg-config Darrick J. Wong
2025-10-16 15:40 ` [PATCH 02/16] libext2fs: use F_GETFL, not F_GETFD, in unixfd_open Darrick J. Wong
2025-10-16 15:40 ` [PATCH 03/16] libext2fs: don't look for O_EXCL in the F_GETFL output Darrick J. Wong
2025-10-16 15:40 ` Darrick J. Wong [this message]
2025-10-16 15:41 ` [PATCH 05/16] libext2fs: the unixfd IO manager shouldn't close its fd Darrick J. Wong
2025-10-16 15:41 ` [PATCH 06/16] fuse2fs: update manpage Darrick J. Wong
2025-10-16 15:41 ` [PATCH 07/16] fuse2fs: quiet down EXT2_ET_RO_FILSYS errors Darrick J. Wong
2025-10-16 15:41 ` [PATCH 08/16] fuse2fs: free global_fs after a failed ext2fs_close call Darrick J. Wong
2025-10-16 15:42 ` [PATCH 09/16] fuse2fs: fix memory corruption when parsing mount options Darrick J. Wong
2025-10-16 15:42 ` [PATCH 10/16] fuse2fs: fix fssetxattr flags updates Darrick J. Wong
2025-10-16 15:42 ` [PATCH 11/16] fuse2fs: fix default acls propagating to non-dir children Darrick J. Wong
2025-10-16 15:42 ` [PATCH 12/16] fuse2fs: don't update atime when reading executable file content Darrick J. Wong
2025-10-16 15:43 ` [PATCH 13/16] fuse2fs: fix in_file_group missing the primary process gid Darrick J. Wong
2025-10-16 15:43 ` [PATCH 14/16] fuse2fs: work around EBUSY discard returns from dm-thinp Darrick J. Wong
2025-10-16 15:43 ` [PATCH 15/16] fuse2fs: check free space when creating a symlink Darrick J. Wong
2025-10-16 15:43 ` [PATCH 16/16] fuse2fs: spot check clean journals Darrick J. Wong
2025-10-20 20:26 ` [PATCH 17/16] fuse2fs: recheck support after replaying journal Darrick J. Wong
2025-10-20 20:26 ` [PATCH 18/16] fuse2fs: make norecovery behavior consistent with the kernel Darrick J. Wong
2025-10-20 20:27 ` [PATCH 19/16] fuse2fs: mount norecovery if main block device is readonly Darrick J. Wong
2025-10-21 13:22 ` [PATCHSET] fuse2fs: round 6 bug fixes Theodore Tso

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=176062915537.3343688.2245819523975318264.stgit@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox