[PATCH] hfsplus: validate extent record length before writing it back

Jiaming Zhang posted 1 patch 1 month, 2 weeks ago
fs/hfsplus/extents.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] hfsplus: validate extent record length before writing it back
Posted by Jiaming Zhang 1 month, 2 weeks ago
__hfsplus_ext_write_extent() writes the cached extent record back into a
B-tree node using fd->entrylength as the length, and fd->entrylength is
derived in __hfs_brec_find() from two on-disk values:

	fd->entrylength = len - keylen;

A crafted image can keep both len and keylen valid but make fd->entrylength
negative (keylen > len). __hfsplus_ext_write_extent() doesn't check
fd->entrylength before consuming it, and hfs_bnode_write() takes the length
as u32, so the negative value turns into a huge one. The copy then reads
data past the end of hip->cached_extents, which is only
sizeof(hfsplus_extent_rec) bytes long, and leaks kernel memory into the
image.

Reject an fd->entrylength that does not match sizeof(hfsplus_extent_rec) in
__hfsplus_ext_write_extent(), mirroring the check already performed in
__hfsplus_ext_read_extent().

Link: https://lore.kernel.org/lkml/cbd7003314c530d4f910eacf019ff80adad6687e.camel@dubeyko.com/
Signed-off-by: Jiaming Zhang <r772577952@gmail.com>
---
 fs/hfsplus/extents.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
index 813e68b8ecd6..eb7c11524d18 100644
--- a/fs/hfsplus/extents.c
+++ b/fs/hfsplus/extents.c
@@ -110,6 +110,8 @@ static int __hfsplus_ext_write_extent(struct inode *inode,
 	} else {
 		if (res)
 			return res;
+		if (fd->entrylength != sizeof(hfsplus_extent_rec))
+			return -EIO;
 		hfs_bnode_write(fd->bnode, hip->cached_extents,
 				fd->entryoffset, fd->entrylength);
 		hip->extent_state &= ~HFSPLUS_EXT_DIRTY;
-- 
2.43.0
Re: [PATCH] hfsplus: validate extent record length before writing it back
Posted by Viacheslav Dubeyko 1 month, 2 weeks ago
On Mon, 2026-08-10 at 17:24 +0800, Jiaming Zhang wrote:
> __hfsplus_ext_write_extent() writes the cached extent record back
> into a
> B-tree node using fd->entrylength as the length, and fd->entrylength
> is
> derived in __hfs_brec_find() from two on-disk values:
> 
> 	fd->entrylength = len - keylen;
> 
> A crafted image can keep both len and keylen valid but make fd-
> >entrylength
> negative (keylen > len). __hfsplus_ext_write_extent() doesn't check
> fd->entrylength before consuming it, and hfs_bnode_write() takes the
> length
> as u32, so the negative value turns into a huge one. The copy then
> reads
> data past the end of hip->cached_extents, which is only
> sizeof(hfsplus_extent_rec) bytes long, and leaks kernel memory into
> the
> image.
> 
> Reject an fd->entrylength that does not match
> sizeof(hfsplus_extent_rec) in
> __hfsplus_ext_write_extent(), mirroring the check already performed
> in
> __hfsplus_ext_read_extent().
> 
> Link:
> https://lore.kernel.org/lkml/cbd7003314c530d4f910eacf019ff80adad6687e.camel@dubeyko.com/
> Signed-off-by: Jiaming Zhang <r772577952@gmail.com>
> ---
>  fs/hfsplus/extents.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
> index 813e68b8ecd6..eb7c11524d18 100644
> --- a/fs/hfsplus/extents.c
> +++ b/fs/hfsplus/extents.c
> @@ -110,6 +110,8 @@ static int __hfsplus_ext_write_extent(struct
> inode *inode,
>  	} else {
>  		if (res)
>  			return res;
> +		if (fd->entrylength != sizeof(hfsplus_extent_rec))
> +			return -EIO;
>  		hfs_bnode_write(fd->bnode, hip->cached_extents,
>  				fd->entryoffset, fd->entrylength);
>  		hip->extent_state &= ~HFSPLUS_EXT_DIRTY;

Looks good.

Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>

Thanks,
Slava.