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

[Viacheslav Dubeyko]

On Wed, 2026-10-21 at 10:56 -0400, Hengyu Liang wrote:
> hfs_inode_setattr() sets i_size and calls hfs_file_truncate() to
> change
> the allocation. If expansion fails, hfs_file_truncate() restores
> i_size
> to the allocated size, but its void return type leaves setattr
> reporting
> success.
> 
> On a freshly formatted 64 MiB HFS volume, ftruncate() to 128 MiB
> returns
> zero while the file remains empty. When the requested size fits
> within
> the volume but exceeds the free space, it also returns zero, leaving
> the file only partially extended.
> 
> Call generic_cont_expand_simple() when growing a file and propagate
> its
> error. Keep truncate_setsize() and hfs_file_truncate() in the
> shrinking
> path. This makes failed extensions report EFBIG or ENOSPC.

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;

        size = inode->i_size + HFS_SB(sb)->alloc_blksz - 1;
        ...     /* existing shrink logic unchanged */
}

It will be great if you could share another patch with the fix.

> 
> Signed-off-by: Hengyu Liang <hengyul@xxxxxxxxxx>
> ---
> Changes in v2:
> - Keep truncate_setsize() and hfs_file_truncate() in the shrinking
> path,
>   as suggested by Viacheslav Dubeyko.
> - Replace the generic/736 and fsx runs with a comparison of the whole
>   xfstests set selected for HFS, as requested.
> 
> v1:
> https://lore.kernel.org/r/20261017160612.3740256-1-hengyul@xxxxxxxxxx
> 
> Compared unpatched and patched v7.3-rc7 with identical KASAN configs.
> All 805 tests selected for HFS by "check -n -g all" were attempted,
> using freshly formatted 2 GiB test and scratch volumes for each case:
>                 passed  failed  not run
>   unpatched       109      27      669
>   v2              109      27      669
> 
> The different results were generic/113, /394, /464 and /795. /113 and
> /464 passed and failed in replays on the unpatched kernel, and the
> /113
> failures are the same existing workqueue warning from
> hfs_file_fsync().
> /795 failed on all three kernels in replays. On the 2 GiB volume,
> /394
> passes without the fix only because ftruncate() returns success after
> a
> partial extension; v1 and v2 report ENOSPC. It passes on all three
> kernels with a 4 GiB volume.
> 
> Also ran 39 functional and 18 recovery cases per kernel on 32/64/128
> MiB
> volumes. Content and read-only remount checks passed on unpatched, v1
> and v2 kernels. All three had the same 12 post-ENOSPC fsck failures;
> recovery fsck checks were clean.
> 
> Functional/recovery cases and generic/394 were repeated on unpatched
> and v2 kernels at the hfs 7.4 merge (dd7b15aeac80), with the same
> results.
> 
>  fs/hfs/inode.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
> index 2aef3c36a150..9bdaa60fc392 100644
> --- a/fs/hfs/inode.c
> +++ b/fs/hfs/inode.c
> @@ -684,8 +684,14 @@ int hfs_inode_setattr(struct mnt_idmap *idmap,
> struct dentry *dentry,
>  		if (error)
>  			return error;
>  
> -		truncate_setsize(inode, attr->ia_size);
> -		hfs_file_truncate(inode);
> +		if (attr->ia_size > inode->i_size) {
> +			error = generic_cont_expand_simple(inode,
> attr->ia_size);
> +			if (error)
> +				return error;
> +		} else {
> +			truncate_setsize(inode, attr->ia_size);
> +			hfs_file_truncate(inode);
> +		}
>  		inode_set_mtime_to_ts(inode,
> inode_set_ctime_current(inode));
>  	}
>  

Looks good.

Reviewed-by: Viacheslav Dubeyko <slava@xxxxxxxxxxx>

Thanks,
Slava.

[1]
https://elixir.bootlin.com/linux/v7.3-rc5/source/fs/hfs/extent.c#L488