Re: [PATCH] hfsplus: fall back to buffered I/O for direct writes past phys_size

[Viacheslav Dubeyko]

On Fri, 2026-10-30 at 00:32 -0400, Hengyu Liang wrote:
> hfsplus_direct_IO() passes direct writes to blockdev_direct_IO(),
> which
> does not go through hfsplus_write_begin(). phys_size only grows by
> the
> block size when hfsplus_get_block() allocates a new block. It starts
> at
> i_size when the inode is read, which does not have to be block
> aligned,
> so after a direct write past the end of the file phys_size is smaller
> than i_size.
> 
> However, at the last close, hfsplus_file_truncate() sees i_size
> larger
> than phys_size and calls hfsplus_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. Commit 5f63ac80aef2 ("hfsplus: fix issue
> of
> direct writes beyond end-of-file") avoids this only for writes that
> start past the end of the file.
> 
> The issue can be reproduced with:
> 
>     truncate -s 32M hfsplus.img
>     mkfs.hfsplus hfsplus.img
>     mount -o loop hfsplus.img /mnt
>     printf 'A%.0s' $(seq 100) > /mnt/f
>     umount /mnt
>     mount -o loop hfsplus.img /mnt
>     xfs_io -d -c 'pwrite -S 0x42 0 1024' /mnt/f
>     umount /mnt
>     mount -o loop hfsplus.img /mnt
>     od -An -tx1 -j 100 -N 924 /mnt/f
> 
> Bytes 100 to 1023 read back as zeros instead of 0x42.
> 
> This patch will make the other direct writes that end 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@xxxxxxxxxxxxxxx
> Signed-off-by: Hengyu Liang <hengyul@xxxxxxxxxx>
> ---
> Slava, you mentioned that you are rebuilding the HFS+ direct I/O path
> [1]. This patch fixes the current code, also for stable, and may
> conflict with that work.
> 
> Writes that start past EOF still expand the file first, as in
> 5f63ac80aef2. Only the other writes that end past phys_size fall
> back.
> 
> Tested in KVM with KASAN: the reproducer above, and all 805 fstests
> on
> HFS+: 10 failures without the patch, 7 with it (generic/521, 617 and
> 760 now pass), no new failures.
> 
> [1]
> https://lore.kernel.org/linux-fsdevel/161fab786f919adb384fbf22dc19f3289bf5f24d.camel@xxxxxxxxxxx/
> 
>  fs/hfsplus/inode.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
> index aed0866499b2..70c13dcdcd8e 100644
> --- a/fs/hfsplus/inode.c
> +++ b/fs/hfsplus/inode.c
> @@ -169,6 +169,15 @@ static ssize_t hfsplus_direct_IO(struct kiocb
> *iocb, struct iov_iter *iter)
>  		end_page = end_off >> PAGE_SHIFT;
>  
>  		invalidate_inode_pages2_range(mapping, start_page,
> end_page);
> +	} else if (iov_iter_rw(iter) == WRITE &&
> +		   end > HFSPLUS_I(inode)->phys_size) {
> +		/*
> +		 * 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,
> +		 * so that the write falls back to a buffered write.
> +		 */
> +		return 0;
>  	}
>  
>  	ret = blockdev_direct_IO(iocb, inode, iter,
> hfsplus_get_block);
> 
> base-commit: a804c5742bff224d4cb4d25dd383554d598be32b

I think this modification will be wiped out by my pathset [1] because
hfsplus_direct_IO() will be completely removed. I am going to renew my
work on this pathset now because 7.4-rc1 has been released already. So,
thanks a lot for the patch but I assume that this issue is already
fixed in the patchset. If not, then let's double check it after my
pathset has been finished.

Thanks,
Slava.

[1]
https://lore.kernel.org/linux-fsdevel/20260914233941.2966421-1-slava@xxxxxxxxxxx/