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

[Viacheslav Dubeyko]

On Mon, 2026-09-28 at 23:03 -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 write parameters before calling it so an
> invalid update is reported as -ERANGE instead of being treated as
> successful.
> 
> Use a reusable B-tree node range helper that takes the find data and
> the expected record size. The helper validates the bnode and tree
> pointers, entry offset and entry length, and ensures that the write
> range fits within the node.
> 
> 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@xxxxxxxxx>
> Changes in v6:
> - Rename hfs_bnode_is_valid_range() to is_hfs_bnode_range_valid().
> - Use size_t for the expected record size.
> - Drop the redundant expected_len validity check.
> - Return -ERANGE for invalid find-data/range parameters.
> - Incorporate the final review feedback from Viacheslav Dubeyko.
> 
> ---
>  fs/hfs/btree.h  | 19 +++++++++++++++++++
>  fs/hfs/extent.c | 10 ++++++++--
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/hfs/btree.h b/fs/hfs/btree.h
> index b4c3f2a31471..1f703f2c53d8 100644
> --- a/fs/hfs/btree.h
> +++ b/fs/hfs/btree.h
> @@ -84,6 +84,25 @@ struct hfs_find_data {
>  	int entryoffset, entrylength;
>  };
>  
> +static inline bool is_hfs_bnode_range_valid(struct hfs_find_data
> *fd,
> +					    size_t expected_len)
> +{
> +	struct hfs_bnode *node;
> +
> +	if (!fd)
> +		return false;
> +
> +	node = fd->bnode;
> +	if (!node || !node->tree)
> +		return false;
> +
> +	if (fd->entryoffset < 0 || fd->entrylength != expected_len)
> +		return false;
> +
> +	return (u64)fd->entryoffset + fd->entrylength <=
> +	       node->tree->node_size;
> +}
> +
>  
>  /* 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..26357086517f 100644
> --- a/fs/hfs/extent.c
> +++ b/fs/hfs/extent.c
> @@ -121,12 +121,18 @@ 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 (!is_hfs_bnode_range_valid(fd,
> sizeof(hfs_extent_rec)))
> +			return -ERANGE;
> +		hfs_bnode_write(fd->bnode, HFS_I(inode)-
> >cached_extents,
> +				fd->entryoffset, fd->entrylength);
>  		HFS_I(inode)->flags &= ~HFS_FLG_EXT_DIRTY;
>  	}
>  	return 0;

Looks good.

Reviewed-by: Viacheslav Dubeyko <slava@xxxxxxxxxxx>

Thanks,
Slava.