lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250717220139.GM2672022@frogsfrogsfrogs>
Date: Thu, 17 Jul 2025 15:01:39 -0700
From: "Darrick J. Wong" <djwong@...nel.org>
To: tytso@....edu
Cc: linux-ext4@...r.kernel.org
Subject: [PATCH 17/8] fuse2fs: fix punch-out range calculation in
 fuse2fs_punch_range

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

In non-iomap mode, generic/008 tries to fzero the first byte of a block
and instead zeroes the entire file:

--- a/tests/generic/008.out      2025-07-15 14:45:14.937058680 -0700
+++ b/tests/generic/008.out.bad        2025-07-16 13:31:03.909315508 -0700
@@ -4,10 +4,7 @@
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 1024/1024 bytes at offset 1024
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-00000000:  00 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  .AAAAAAAAAAAAAAA
-00000010:  41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41  AAAAAAAAAAAAAAAA
-*
-00000400:  42 42 42 42 42 42 42 42 42 42 42 42 42 42 42 42  BBBBBBBBBBBBBBBB
+00000000:  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
 *
 read 2048/2048 bytes at offset 0
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

Looking at the fuse2fs debugging output, the reason why is obvious:

FUSE2FS (sda): op_fallocate: ino=50 mode=0x10 start=0x0 end=0x1
FUSE2FS (sda): fuse2fs_punch_range: ino=50 mode=0x11 offset=0x0 len=0x1 start=0 end=0

start and end are both zero, so we call ext2fs_punch with those
arguments.  ext2fs_punch interprets [start, end] as a closed interval
and removes block 0, which is not what we asked for!

The computation of end is also too subtle -- the dividend is the
expression (0 + 1 - 4096) which produces a negative number because off_t
is defined to be long long, at least on amd64 Linux.  We rely on the
behavior that dividing a negative dividend by a positive divisor
produces a quotient of zero.

Really what we should do here is round offset up to the next fsblock
and offset+len down to the nearest fsblock.  The first quantity is the
first byte of the range to punch and the second quantity is the next
byte past the range to punch.  Using those as the basis to compute start
and end, the punch should only happen if start < end, and we should pass
[start, end - 1] to ext2fs_punch because it expects a closed interval.

Improve the comments here so that I don't have to work all this out
again the next time I read through here.

Cc: <linux-ext4@...r.kernel.org> # v1.43
Fixes: 81cbf1ef4f5dab ("misc: add fuse2fs, a FUSE server for e2fsprogs")
Signed-off-by: "Darrick J. Wong" <djwong@...nel.org>
---
 misc/fuse2fs.c |   49 +++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/misc/fuse2fs.c b/misc/fuse2fs.c
index 6155dff6645ff6..cee9e657c36767 100644
--- a/misc/fuse2fs.c
+++ b/misc/fuse2fs.c
@@ -124,6 +124,28 @@
 
 static ext2_filsys global_fs; /* Try not to use this directly */
 
+static inline uint64_t round_up(uint64_t b, unsigned int align)
+{
+	unsigned int m;
+
+	if (align == 0)
+		return b;
+	m = b % align;
+	if (m)
+		b += align - m;
+	return b;
+}
+
+static inline uint64_t round_down(uint64_t b, unsigned int align)
+{
+	unsigned int m;
+
+	if (align == 0)
+		return b;
+	m = b % align;
+	return b - m;
+}
+
 #define dbg_printf(fuse2fs, format, ...) \
 	while ((fuse2fs)->debug) { \
 		printf("FUSE2FS (%s): " format, (fuse2fs)->shortdev, ##__VA_ARGS__); \
@@ -4095,11 +4117,18 @@ static int punch_helper(struct fuse_file_info *fp, int mode, off_t offset,
 	if (!(mode & FL_KEEP_SIZE_FLAG))
 		return -EINVAL;
 
-	/* Punch out a bunch of blocks */
-	start = FUSE2FS_B_TO_FSB(ff, offset);
-	end = (offset + len - fs->blocksize) / fs->blocksize;
-	dbg_printf(ff, "%s: ino=%d mode=0x%x start=%llu end=%llu\n", __func__,
-		   fh->ino, mode, start, end);
+	/*
+	 * Unmap out all full blocks in the middle of the range being punched.
+	 * The start of the unmap range should be the first byte of the first
+	 * fsblock that starts within the range.  The end of the range should
+	 * be the next byte after the last fsblock to end in the range.
+	 */
+	start = FUSE2FS_B_TO_FSBT(ff, round_up(offset, fs->blocksize));
+	end = FUSE2FS_B_TO_FSBT(ff, round_down(offset + len, fs->blocksize));
+
+	dbg_printf(ff,
+ "%s: ino=%d mode=0x%x offset=0x%jx len=0x%jx start=0x%llx end=0x%llx\n",
+		   __func__, fh->ino, mode, offset, len, start, end);
 
 	err = fuse2fs_read_inode(fs, fh->ino, &inode);
 	if (err)
@@ -4120,10 +4149,14 @@ static int punch_helper(struct fuse_file_info *fp, int mode, off_t offset,
 	if (err)
 		return translate_error(fs, fh->ino, err);
 
-	/* Unmap full blocks in the middle */
-	if (start <= end) {
+	/*
+	 * Unmap full blocks in the middle, which is to say that start - end
+	 * must be at least one fsblock.  ext2fs_punch takes a closed interval
+	 * as its argument, so we pass [start, end - 1].
+	 */
+	if (start < end) {
 		err = ext2fs_punch(fs, fh->ino, EXT2_INODE(&inode),
-				   NULL, start, end);
+				   NULL, start, end - 1);
 		if (err)
 			return translate_error(fs, fh->ino, err);
 	}

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ