Re: [PATCH v2] hfs: report errors when extending files with setattr

[Hengyu Liang]

Hi Slava,

On Wed, Oct 21, 2026 at 02:22:41PM -0700, Viacheslav Dubeyko wrote:
> I think now this code [1] becomes dead. And we need to rework it:
>
> void hfs_file_truncate(struct inode *inode)
> {
>         ...
>         hfs_dbg(...);
>
>         if (WARN_ON_ONCE(inode->i_size > HFS_I(inode)->phys_size))
>                 return;
>         if (inode->i_size == HFS_I(inode)->phys_size)
>                 return;
[...]
> It will be great if you could share another patch with the fix.

Thanks for the review. I made this change and ran the whole xfstests
set, but the grow path is still reached. The WARN_ON_ONCE() fires under
fsstress in generic/013, 076, 476, 650, 750 and 795, every time from
close() through hfs_file_release().

It comes from direct writes:

1. When an inode is read, phys_size is set to i_size, which does not
   have to be block aligned, e.g. 100.
2. A direct write does not go through cont_write_begin(). When it
   needs a new block, hfs_get_block() only adds s_blocksize to
   phys_size. A direct write of 512 bytes at offset 512 makes phys_size
   612 and i_size 1024.
3. At the last close, hfs_file_truncate() sees i_size > phys_size and
   calls hfs_write_begin(). cont_write_begin() then zeroes
   [phys_size, i_size) in the page cache, which overwrites the data
   that the direct write has just written.

So today this path loses data, and removing it would only replace that
with the warning, while phys_size stays behind i_size.

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

On a804c5742bff, with and without the v2 patch, the last 412 bytes
read back as zeros instead of 0x42, and fsck.hfs reports no error.
With the cleanup applied, the data is intact but the warning fires.

HFS+ has the same problem. hfsplus_file_truncate() has the same grow
path, and commit 5f63ac80aef2 ("hfsplus: fix issue of direct writes
beyond end-of-file") only covers writes that start past EOF. A direct
write of 1024 bytes at offset 0 to the same 100-byte file loses 924
bytes on HFS+.

I think the direct write path should be fixed first, so that i_size
can no longer get past phys_size. Then the grow path really is dead
and your cleanup is right. The simplest way I see is what
fat_direct_IO() does: if a direct write would end past phys_size,
return 0 so that it falls back to a buffered write, which zeroes and
accounts through cont_write_begin().

Would that be fine with you? If so, I will send that fix for HFS with
your cleanup on top, and a similar fix for HFS+.

Thanks,
Hengyu