Re: [PATCH v2] hfsplus: validate inline xattr record size against entrylength

[Viacheslav Dubeyko]

On Thu, 2026-09-24 at 07:07 +0000, Hui Peng wrote:
> In __hfsplus_getxattr(), record_length is read from the on-disk
> hfsplus_attr_inline_data header and only checked against
> HFSPLUS_MAX_INLINE_DATA_SIZE without verifying that fd.entrylength is
> large enough to hold the inline header and record_length bytes of
> raw_bytes. A corrupted attribute B-tree node where record_length
> exceeds
> fd.entrylength causes hfs_bnode_read() to read past the end of the B-
> tree
> record (and potentially across the bnode boundary).
> 
> Validate fd.entrylength before reading xattr_record_type, length, and
> raw_bytes.
> 
> Tested in QEMU against Linux 7.3.0-rc3 by mounting a crafted HFS+
> image
> containing an inline xattr record where record_length (100) exceeded
> fd.entrylength (4): on the unfixed kernel, __hfsplus_getxattr() read
> past
> the end of the B-tree record; whereas with this fix applied,
> __hfsplus_getxattr() rejects the malformed record with "invalid xattr
> record size" (-EIO).
> 
> Fixes: 127e5f5ae51e ("hfsplus: rework functionality of getting,
> setting and deleting of extended attributes")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@xxxxxxxxx>
> ---
> Changes in v2:
> - Drop the hidden_dir cleanup hunk (already covered by Deepanshu
>   Kartikey's patch series) and focus solely on the
> __hfsplus_getxattr()
>   entrylength validation, as requested by Viacheslav Dubeyko.
> 
>  fs/hfsplus/xattr.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
> index 21a1c196c71f..10aae766ea42 100644
> --- a/fs/hfsplus/xattr.c
> +++ b/fs/hfsplus/xattr.c
> @@ -649,15 +649,28 @@ ssize_t __hfsplus_getxattr(struct inode *inode,
> const char *name,
>  		goto out;
>  	}
>  
> +	if (fd.entrylength < sizeof(xattr_record_type)) {
> +		pr_err("invalid xattr record size\n");
> +		res = -EIO;
> +		goto out;
> +	}
>  	hfs_bnode_read(fd.bnode, &xattr_record_type,
>  			fd.entryoffset, sizeof(xattr_record_type));

We need to incorporate likewise checks in the hfs_bnode_read(). Frankly
speaking, I don't think that we cannot read __be32. But if we cannot,
then only hfs_bnode_read() knows about the failure.

>  	record_type = be32_to_cpu(xattr_record_type);
>  	if (record_type == HFSPLUS_ATTR_INLINE_DATA) {
> +		if (fd.entrylength < offsetof(struct
> hfsplus_attr_inline_data,
> +					      raw_bytes)) {

The struct hfsplus_attr_inline_data defines maximum possible size of
buffer (HFSPLUS_MAX_INLINE_DATA_SIZE). But its doesn't mean that every
record is 3802 bytes in size. This check should fail always.

> +			pr_err("invalid xattr record size\n");
> +			res = -EIO;
> +			goto out;
> +		}
>  		record_length = hfs_bnode_read_u16(fd.bnode,
>  				fd.entryoffset +
>  				offsetof(struct
> hfsplus_attr_inline_data,
>  				length));
> -		if (record_length > HFSPLUS_MAX_INLINE_DATA_SIZE) {
> +		if (record_length > HFSPLUS_MAX_INLINE_DATA_SIZE ||
> +		    offsetof(struct hfsplus_attr_inline_data,
> raw_bytes) +
> +		    record_length > fd.entrylength) {

Frankly speaking, I don't follow this check. It looks pretty
complicated.

Thanks,
Slava.

>  			pr_err("invalid xattr record size\n");
>  			res = -EIO;
>  			goto out;