Re: [PATCH v4] hfs: handle extent B-tree write errors

[Viacheslav Dubeyko <slava@dubeyko.com>]

On Wed, 2026-09-23 at 21:16 -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 errors returned by hfs_brec_insert() and only clear the
> extent flags after a successful insertion.
> 
> When updating an existing extent record, hfs_bnode_write() returns
> void. Validate the extent record size and use a reusable B-tree node
> range helper to reject invalid write parameters before calling
> hfs_bnode_write(). This prevents an invalid update from being treated
> as successful and avoids clearing HFS_FLG_EXT_DIRTY in that case.
> 
> Negative-path testing in QEMU confirmed that an insertion error is
> propagated to the caller. Testing the existing-record path also
> confirmed that invalid write parameters are rejected before
> HFS_FLG_EXT_DIRTY is cleared.
> 
> Signed-off-by: Davy Felipe <davyfelipe34@gmail.com>
> 
> Sorry, I missed your suggestion about factoring the validation into a
> reusable helper in v3. This revision addresses it.
> 
> Changes in v4:
> - Factor B-tree node range validation into a reusable helper, as
>   suggested by Viacheslav Dubeyko.
> - Keep the extent-record size check local to the extent write path.
> - Preserve the error propagation and validation behavior from v3.
> 
> ---
>  fs/hfs/btree.h  |  7 +++++++
>  fs/hfs/extent.c | 12 ++++++++++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/hfs/btree.h b/fs/hfs/btree.h
> index b4c3f2a31471..576412e6901e 100644
> --- a/fs/hfs/btree.h
> +++ b/fs/hfs/btree.h
> @@ -84,6 +84,13 @@ struct hfs_find_data {
>  	int entryoffset, entrylength;
>  };
>  
> +static inline bool hfs_bnode_is_valid_range(struct hfs_bnode *node,
> +					    int off, int len)

You can use struct hfs_find_data *fd. It can make argument list
shorter. We need to find the node before read/write. So, I think we
should have hfs_find_data available.

> +{
> +	return off >= 0 && len > 0 &&
> +	       (u64)off + len <= node->tree->node_size;

Maybe, it makes sense to check that node->tree pointers are valid?

> +}
> +
>  
>  /* btree.c */
>  extern struct hfs_btree *hfs_btree_open(struct super_block *sb, u32
> id,
> diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
> index f066a99a863b..ece782205b47 100644
> --- a/fs/hfs/extent.c
> +++ b/fs/hfs/extent.c
> @@ -121,12 +121,20 @@ 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)
>  			return res;
> -		hfs_bnode_write(fd->bnode, HFS_I(inode)-
> >cached_extents, fd->entryoffset, fd->entrylength);
> +		if (fd->entrylength != sizeof(hfs_extent_rec) ||
> +		    !hfs_bnode_is_valid_range(fd->bnode, fd-
> >entryoffset,
> +					      fd->entrylength))

I think we can share with hfs_bnode_is_valid_range() expected size
sizeof(hfs_extent_rec) and hfs_bnode_is_valid_range() will be able to
check the fd->entrylength. What do you think?

Thanks,
Slava.

> +			return -EIO;
> +		hfs_bnode_write(fd->bnode, HFS_I(inode)-
> >cached_extents,
> +				fd->entryoffset, fd->entrylength);
>  		HFS_I(inode)->flags &= ~HFS_FLG_EXT_DIRTY;
>  	}
>  	return 0;