[PATCH 0/2] hfs: fall back to buffered I/O for direct writes past phys_size

[PATCH 0/2] hfs: fall back to buffered I/O for direct writes past phys_size

[Hengyu Liang]

Hi Slava,

As discussed in the setattr thread [1], a direct write that ends past
phys_size loses data at the last close, because hfs_file_truncate()
zeroes the file from phys_size up to i_size. Patch 1 makes such writes
fall back to buffered I/O, as FAT does. Patch 2 is the
hfs_file_truncate() cleanup you suggested.

The series applies on top of "hfs: report errors when extending files
with setattr" v2 [2], which patch 2 needs. The HFS+ fix is sent
separately.

Tested in KVM with KASAN, all 805 fstests on HFS with that v2 applied:
26 failures without the series, 14 with patch 1 and 15 with both, no
new failures. The extra one, generic/589, fails on and off on all three
kernels with "invalid MDB drNxtCNID", the next CNID issue from the CNID
thread.

[1] https://lore.kernel.org/linux-fsdevel/161fab786f919adb384fbf22dc19f3289bf5f24d.camel@dubeyko.com/
[2] https://lore.kernel.org/linux-fsdevel/20261021145627.288953-1-hengyul@cs.unc.edu/

Hengyu Liang (2):
  hfs: fall back to buffered I/O for direct writes past phys_size
  hfs: remove the dead grow path from hfs_file_truncate()

 fs/hfs/extent.c | 18 ++----------------
 fs/hfs/inode.c  | 10 ++++++++++
 2 files changed, 12 insertions(+), 16 deletions(-)


base-commit: a804c5742bff224d4cb4d25dd383554d598be32b
prerequisite-patch-id: 737ea6125c4a7fa08a4deb38cf33d27fb791a812
-- 
2.53.0

[PATCH 1/2] hfs: fall back to buffered I/O for direct writes past phys_size

[Hengyu Liang]

hfs_direct_IO() passes direct writes to blockdev_direct_IO(), which
does not go through hfs_write_begin(). When such a write needs a new
block, hfs_get_block() only adds the block size to phys_size. phys_size
starts at i_size when the inode is read, which does not have to be
block aligned, so after the write phys_size is smaller than i_size.

However, at the last close, hfs_file_truncate() sees i_size larger than
phys_size and calls hfs_write_begin() at i_size, which zeroes the file
from phys_size up to i_size. As of now, part of the data that a direct
write has just written past the old end of the file is lost, and fsck
reports no error.

The issue can be reproduced with:

    truncate -s 32M hfs.img
    mkfs.hfsplus -h hfs.img
    mount -t hfs -o loop hfs.img /mnt
    printf 'A%.0s' $(seq 100) > /mnt/f
    umount /mnt
    mount -t hfs -o loop hfs.img /mnt
    xfs_io -d -c 'pwrite -S 0x42 512 512' /mnt/f
    umount /mnt
    mount -t hfs -o loop hfs.img /mnt
    od -An -tx1 -j 512 -N 512 /mnt/f

The last 412 bytes read back as zeros instead of 0x42.

This patch will make a direct write that ends past phys_size fall back
to a buffered write, as fat_direct_IO() and affs_direct_IO() do.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
---
 fs/hfs/inode.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index d9abab2a1e61..d476a16f4ef9 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -139,6 +139,16 @@ static ssize_t hfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
 	size_t count = iov_iter_count(iter);
 	ssize_t ret;
 
+	/*
+	 * blockdev_direct_IO() does not go through ->write_begin(), so it
+	 * neither zeroes the file from phys_size up to the write nor keeps
+	 * phys_size in step with i_size. Return 0 for a write that ends
+	 * past phys_size, so that it falls back to a buffered write.
+	 */
+	if (iov_iter_rw(iter) == WRITE &&
+	    iocb->ki_pos + count > HFS_I(inode)->phys_size)
+		return 0;
+
 	ret = blockdev_direct_IO(iocb, inode, iter, hfs_get_block);
 
 	/*
-- 
2.53.0

[PATCH 2/2] hfs: remove the dead grow path from hfs_file_truncate()

[Hengyu Liang]

hfs_file_truncate() grows the file through hfs_write_begin() when
i_size is larger than phys_size. That case no longer happens.

phys_size is set to i_size when the inode is read. After that, i_size
only grows through hfs_write_begin(), which grows phys_size first, or
through a direct write that ends within phys_size.
"hfs: report errors when extending files with setattr" made
hfs_inode_setattr() extend files with generic_cont_expand_simple(),
and "hfs: fall back to buffered I/O for direct writes past phys_size"
made direct writes past phys_size go through hfs_write_begin() too.

This patch removes the grow path, and warns and returns if i_size is
ever larger than phys_size.

Suggested-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
---
 fs/hfs/extent.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
index 26357086517f..8d04b7c0911e 100644
--- a/fs/hfs/extent.c
+++ b/fs/hfs/extent.c
@@ -491,23 +491,9 @@ void hfs_file_truncate(struct inode *inode)
 	hfs_dbg("ino %llu, phys_size %llu -> i_size %llu\n",
 		inode->i_ino, (long long)HFS_I(inode)->phys_size,
 		inode->i_size);
-	if (inode->i_size > HFS_I(inode)->phys_size) {
-		struct address_space *mapping = inode->i_mapping;
-		void *fsdata = NULL;
-		struct folio *folio;
-
-		/* XXX: Can use generic_cont_expand? */
-		size = inode->i_size - 1;
-		res = hfs_write_begin(NULL, mapping, size + 1, 0, &folio,
-				&fsdata);
-		if (!res) {
-			res = generic_write_end(NULL, mapping, size + 1, 0, 0,
-					folio, fsdata);
-		}
-		if (res)
-			inode->i_size = HFS_I(inode)->phys_size;
+	if (WARN_ON_ONCE(inode->i_size > HFS_I(inode)->phys_size))
 		return;
-	} else if (inode->i_size == HFS_I(inode)->phys_size)
+	if (inode->i_size == HFS_I(inode)->phys_size)
 		return;
 	size = inode->i_size + HFS_SB(sb)->alloc_blksz - 1;
 	blk_cnt = size / HFS_SB(sb)->alloc_blksz;
-- 
2.53.0

[syzbot ci] Re: hfs: fall back to buffered I/O for direct writes past phys_size

[syzbot ci]

syzbot ci has tested the following series

[v1] hfs: fall back to buffered I/O for direct writes past phys_size
https://lore.kernel.org/all/20261030042903.4160506-1-hengyul@cs.unc.edu
* [PATCH 1/2] hfs: fall back to buffered I/O for direct writes past phys_size
* [PATCH 2/2] hfs: remove the dead grow path from hfs_file_truncate()

and found the following issue:
WARNING in hfs_file_truncate

Full report is available here:
https://ci.syzbot.org/series/f8323fc4-a62a-40ea-9106-bff8391323a3

***

WARNING in hfs_file_truncate

tree:      axboe
URL:       https://kernel.googlesource.com/pub/scm/linux/kernel/git/axboe/linux.git
base:      a804c5742bff224d4cb4d25dd383554d598be32b
arch:      amd64
compiler:  Debian clang version 22.1.8 (1~deb13u4), Debian LLD 22.1.8
config:    https://ci.syzbot.org/builds/734131df-1170-4dc7-a0ec-85ffb659aeec/config
syz repro: https://ci.syzbot.org/findings/95638e3e-6c06-4f46-a68b-e468dc287d1e/syz_repro

------------[ cut here ]------------
inode->i_size > HFS_I(inode)->phys_size
WARNING: fs/hfs/extent.c:494 at hfs_file_truncate+0x7cf/0xa50 fs/hfs/extent.c:494, CPU#1: syz.2.19/5839
Modules linked in:
CPU: 1 UID: 0 PID: 5839 Comm: syz.2.19 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:hfs_file_truncate+0x7cf/0xa50 fs/hfs/extent.c:494
Code: 8b 05 ed eb db 11 48 3b 84 24 20 01 00 00 75 1f 48 8d 65 d8 5b 41 5c 41 5d 41 5e 41 5f 5d e9 88 09 33 09 cc e8 02 10 0b ff 90 <0f> 0b 90 eb 9b e8 67 00 30 09 44 89 e9 80 e1 07 80 c1 03 38 c1 0f
RSP: 0018:ffffc900034afaa0 EFLAGS: 00010293
RAX: ffffffff82bca45e RBX: 1ffff92000695f68 RCX: ffff88810ca50000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000852
RBP: ffffc900034afc08 R08: ffffc900034afb97 R09: 0000000000000000
R10: ffffc900034afb60 R11: fffff52000695f73 R12: 0000000000000000
R13: ffff88816b59b100 R14: dffffc0000000000 R15: 0000000000000852
FS:  00007f4cc5ac36c0(0000) GS:ffff8882a7c97000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000560c8ef2a008 CR3: 00000001bd922000 CR4: 00000000000006f0
Call Trace:
 <TASK>
 hfs_inode_setattr+0x4d0/0x730 fs/hfs/inode.c:698
 notify_change+0xbba/0xea0 fs/attr.c:556
 do_truncate+0x1c2/0x250 fs/open.c:68
 do_ftruncate+0x4d4/0x580 fs/open.c:194
 ksys_ftruncate fs/open.c:206 [inline]
 __do_sys_ftruncate fs/open.c:211 [inline]
 __se_sys_ftruncate fs/open.c:209 [inline]
 __x64_sys_ftruncate+0x8f/0xe0 fs/open.c:209
 do_syscall_x64 arch/x86/entry/syscall_64.c:61 [inline]
 do_syscall_64+0x166/0x520 arch/x86/entry/syscall_64.c:84
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f4cc4b9f559
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f4cc5ac3028 EFLAGS: 00000246 ORIG_RAX: 000000000000004d
RAX: ffffffffffffffda RBX: 00007f4cc4e25fa0 RCX: 00007f4cc4b9f559
RDX: 0000000000000000 RSI: 0000000000000852 RDI: 0000000000000004
RBP: 00007f4cc4c36b20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f4cc4e26038 R14: 00007f4cc4e25fa0 R15: 00007ffcf07cc2c8
 </TASK>


***

If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
  Tested-by: syzbot@syzkaller.appspotmail.com

---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.

To test a fix for this bug, please reply with `#syz test`
(on a separate line) and attach the patch to the email.

Notes:
- The patch will be applied on top of the tested series (as an
  incremental fix).
- To test a new version of the whole series, please send it directly
  to syzbot@lists.linux.dev.
- Arguments like custom git repos and branches are not supported.