Re: [PATCH] hfs: propagate extent B-tree insertion errors

[Viacheslav Dubeyko <slava@dubeyko.com>]

On Sun, 2026-09-20 at 13:02 -0300, Davy Felipe wrote:
> hfs_brec_insert() may fail while inserting a new extent record, but
> __hfs_ext_write_extent() currently ignores its return value and
> clears
> HFS_FLG_EXT_DIRTY and HFS_FLG_EXT_NEW as if the insertion had
> succeeded.
> 
> Propagate the error returned by hfs_brec_insert() and only clear the
> extent flags after a successful insertion.
> 
> Fault injection of -EIO into the extent B-tree insertion path
> confirmed
> that __hfs_ext_write_extent() returned success despite the insertion
> failure. With the error propagated, the failure is returned to the
> caller instead.

How HFS driver behaves when we return error from hfs_brec_insert()? Are
we capable to properly process it? I assume that everything should work
well. But have you tested failure use-case? 

> 
> Signed-off-by: Davy Felipe <davyfelipe34@gmail.com>
> ---
>  fs/hfs/extent.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
> index f066a99a863b..77ba0f63fcb9 100644
> --- a/fs/hfs/extent.c
> +++ b/fs/hfs/extent.c
> @@ -121,7 +121,10 @@ static int __hfs_ext_write_extent(struct inode
> *inode, struct hfs_find_data *fd)
>  		res = hfs_bmap_reserve(fd->tree, fd->tree->depth +
> 1);
>  		if (res)
>  			return res;
> -		hfs_brec_insert(fd, HFS_I(inode)->cached_extents,
> sizeof(hfs_extent_rec));
> +		res = hfs_brec_insert(fd, HFS_I(inode)-
> >cached_extents,
> +				      sizeof(hfs_extent_rec));
> +		if (res)
> +			return res;
>  		HFS_I(inode)->flags &=
> ~(HFS_FLG_EXT_DIRTY|HFS_FLG_EXT_NEW);
>  	} else {
>  		if (res)

We ignore potential errors from hfs_bnode_write(). Should we start
processing it too?

	} else {
		if (res)
			return res;
		hfs_bnode_write(fd->bnode, HFS_I(inode)-
>cached_extents, fd->entryoffset, fd->entrylength);
		HFS_I(inode)->flags &= ~HFS_FLG_EXT_DIRTY;
	}

Thanks,
Slava.