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

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

[Davy Felipe]

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.

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)
-- 
2.55.0

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

[Viacheslav Dubeyko]

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.

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

[Davy Felipe]

__hfs_ext_write_extent() does not report all failures while updating
the extents B-tree.

When inserting a new extent record, the return value of
hfs_brec_insert() is ignored and HFS_FLG_EXT_DIRTY and HFS_FLG_EXT_NEW
are cleared even if the insertion fails.

When updating an existing extent record, hfs_bnode_write() returns
void, so its caller cannot detect a rejected write. Validate the extent
record size and node range before calling hfs_bnode_write().

Propagate errors returned by hfs_brec_insert() and return -EIO for an
invalid existing extent record. Only clear the extent dirty flags after
a successful operation.

Fault injection confirmed both failure paths. Insertion errors are
propagated to the caller, and invalid existing-record writes are
rejected before hfs_bnode_write() without clearing the dirty state.

Signed-off-by: Davy Felipe <davyfelipe34@gmail.com>

Changes in v2:
- Validate the existing extent record size and node range before
  calling hfs_bnode_write(), following review feedback.
- Return -EIO without clearing HFS_FLG_EXT_DIRTY when validation
  fails.
- Fault-injection tested the existing-record failure path. Before the
  change, hfs_bnode_write() rejected an invalid offset internally but
  __hfs_ext_write_extent() continued and cleared the dirty flag. With
  v2, the invalid write is rejected before hfs_bnode_write().

---
 fs/hfs/extent.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
index f066a99a863b..13426503fbb3 100644
--- a/fs/hfs/extent.c
+++ b/fs/hfs/extent.c
@@ -121,12 +121,21 @@ 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) ||
+		    fd->entryoffset < 0 ||
+		    (u64)fd->entryoffset + fd->entrylength >
+			    fd->tree->node_size)
+			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;
-- 
2.55.0