Re: [PATCH] ocfs2: validate xattr entry count in ocfs2_xattr_list_entries

[Heming Zhao]

On Tue, Nov 11, 2025 at 01:08:31PM +0530, Deepanshu Kartikey wrote:
> Add validation of xattr entry count before accessing entries to prevent
> out-of-bounds array access and use-after-free bugs. A corrupted
> filesystem with an invalid xh_count value can cause the loop to access
> memory beyond the allocated block, potentially reaching freed memory
> pages.
> 
> The validation calculates the maximum number of entries that can fit in
> the available space and rejects counts that exceed this limit. This
> prevents the subsequent loop from accessing invalid memory addresses.
> 
> Without this check, the code directly uses xh_count from disk in array
> indexing operations like &header->xh_entries[i], which can point outside
> the block when xh_count is corrupted, triggering KASAN use-after-free
> detection.
> 
> Reported-by: syzbot+ab0ad25088673470d2d9@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=ab0ad25088673470d2d9
> Tested-by: syzbot+ab0ad25088673470d2d9@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Deepanshu Kartikey <kartikey406@xxxxxxxxx>
> ---
>  fs/ocfs2/xattr.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index d70a20d29e3e..2caf63c6206e 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -928,8 +928,23 @@ static int ocfs2_xattr_list_entries(struct inode *inode,
>  	size_t result = 0;
>  	int i, type, ret;
>  	const char *name;
> +	u16 count;
> +	size_t max_entries;
> +	struct super_block *sb = inode->i_sb;
> +
> +	count = le16_to_cpu(header->xh_count);
> +	max_entries = (sb->s_blocksize - sizeof(struct ocfs2_xattr_header)) /
> +		       sizeof(struct ocfs2_xattr_entry);
>  
> -	for (i = 0 ; i < le16_to_cpu(header->xh_count); i++) {
> +	if (count > max_entries) {
> +		ocfs2_error(sb,
> +			    "xattr entry count %u exceeds maximum %zu in inode %llu\n",
> +			    count, max_entries,
> +			    (unsigned long long)OCFS2_I(inode)->ip_blkno);
> +		return -EUCLEAN;

The meaning of EUCLEAN doesn't align with the code logic; it appears that EFAULT
or ENOMEM would be more appropriate.
The others look good to me.

Thanks,
Heming
> +	}
> +
> +	for (i = 0; i < count; i++) {
>  		struct ocfs2_xattr_entry *entry = &header->xh_entries[i];
>  		type = ocfs2_xattr_get_type(entry);
>  		name = (const char *)header +
> -- 
> 2.43.0
> 
>