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

Viacheslav Dubeyko <[email protected]>
Newsgroups org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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/[email protected]/
> Signed-off-by: Jiaming Zhang <[email protected]>
> ---
>  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 <[email protected]>

Thanks,
Slava.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.