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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ab0ad25088673470d2d9
Tested-by: syzbot+ab0ad25088673470d2d9@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 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;
+	}
+
+	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

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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ab0ad25088673470d2d9
> Tested-by: syzbot+ab0ad25088673470d2d9@syzkaller.appspotmail.com
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
>  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
> 
>

Hi Heming,

Thanks for the review. I will send Patch v2 soon with the required changes.

Thanks

Hi Heming,

Thank you for reviewing the patch! I've sent v2, but I used -EFSCORRUPTED
instead of -EFAULT/-ENOMEM. Here's my reasoning:

This code is detecting on-disk filesystem corruption (invalid xh_count
value from disk), not a memory error or allocation failure. Using
-EFSCORRUPTED:

1. Matches the existing error handling in ocfs2_xattr_find_entry() for
   similar xattr corruption detection
2. Provides clearer semantics (corruption vs generic fault)
3. Is specifically designed for filesystem corruption cases

I'm happy to change it to -EFAULT if you still prefer, but wanted to
explain my reasoning. What do you think?

Thanks

Deepanshu

On Mon, Nov 17, 2025 at 12:08:43PM +0530, Deepanshu Kartikey wrote:
> Hi Heming,
> 
> Thank you for reviewing the patch! I've sent v2, but I used -EFSCORRUPTED
> instead of -EFAULT/-ENOMEM. Here's my reasoning:
> 
> This code is detecting on-disk filesystem corruption (invalid xh_count
> value from disk), not a memory error or allocation failure. Using
> -EFSCORRUPTED:
> 
> 1. Matches the existing error handling in ocfs2_xattr_find_entry() for
>    similar xattr corruption detection
> 2. Provides clearer semantics (corruption vs generic fault)
> 3. Is specifically designed for filesystem corruption cases
> 
> I'm happy to change it to -EFAULT if you still prefer, but wanted to
> explain my reasoning. What do you think?
> 
> Thanks
> 
> Deepanshu

I've noticed that you frequently use separate emails to explain the review
comments. I advise against this practice, as this style loses context and makes
it difficult for other reviewers to follow what's happening.

Another piece of info (which you may already know) is that the kernel mailing
list prefers inline and bottom posting, and discourages top posting.

Thanks
Heming