[PATCH v4 0/2] ocfs2: validate xattr metadata bounds

[PATCH v4 0/2] ocfs2: validate xattr metadata bounds

[Cen Zhang]

Hi,

This v4 rebases the series on current torvalds/master and reworks the
inline xattr part on top of the newly merged
ocfs2_xattr_ibody_lookup_header() helper.

Patch 1 extends the shared ibody helper with xattr entry name/value
bounds checks and calls it from ocfs2_validate_inode_block().

Patch 2 validates non-indexed external xattr blocks from
ocfs2_validate_xattr_block(). Indexed xattr buckets do not pass through
that validator, so their header and per-entry bounds are checked after
bucket ECC verification in ocfs2_read_xattr_bucket().

Both issues were reproduced with crafted OCFS2 images under KASAN.

Changes since v3:
- Rebase on current torvalds/master.
- Build on the newly merged ocfs2_xattr_ibody_lookup_header() helper
  instead of adding a duplicate inline-header helper.
- Keep indexed bucket xh_entries[] bounded by the first bucket block,
  while checking name/value offsets against the addressed bucket block.

Cen Zhang (2):
  ocfs2: validate inline xattrs during inode block validation
  ocfs2: validate external xattr entries when reading metadata

 fs/ocfs2/inode.c |   4 +
 fs/ocfs2/xattr.c | 192 +++++++++++++++++++++++++++++++++++++++++------
 fs/ocfs2/xattr.h |   2 +
 3 files changed, 177 insertions(+), 21 deletions(-)

-- 
2.43.0

[PATCH v4 1/2] ocfs2: validate inline xattrs during inode block validation

[Cen Zhang]

ocfs2_validate_inode_block() verifies a dinode before OCFS2 users walk
metadata from it, but inline xattr metadata is still checked only in
some consumers. The current ibody helper validates the inline header
placement and entry count before deriving the header from
i_xattr_inline_size, but it does not reject entry name/value bounds from
inode block validation.

Extend the shared ibody helper with name/value bounds checks and call it
from ocfs2_validate_inode_block(). Keep the get/list paths using the same
helper before they derive pointers from i_xattr_inline_size so callers
with an already-loaded dinode still get the same corruption check.

Reject corrupted inline xattr metadata before ocfs2_xattr_ibody_get() or
listxattr() can walk past the inline storage.

Validation reproduced this kernel report:
BUG: KASAN: use-after-free in ocfs2_xattr_find_entry+0x5a/0x170
Read of size 2 at addr ffff8881242a2000 by task python3/529
Call Trace:
  dump_stack_lvl+0x66/0xa0
  print_report+0xce/0x630
  kasan_report+0xe0/0x110
  ocfs2_xattr_find_entry+0x5a/0x170
  ocfs2_xattr_get_nolock+0x20a/0x820
  ocfs2_xattr_get+0x10c/0x1e0
  __vfs_getxattr+0xe2/0x130
  vfs_getxattr+0x185/0x1b0

Fixes: cf1d6c763fbc ("ocfs2: Add extended attribute support")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 fs/ocfs2/inode.c |   4 ++
 fs/ocfs2/xattr.c | 112 +++++++++++++++++++++++++++++++++++++----------
 fs/ocfs2/xattr.h |   2 +
 3 files changed, 95 insertions(+), 23 deletions(-)

diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
index 662dbc845b8b..815bf3f659da 100644
--- a/fs/ocfs2/inode.c
+++ b/fs/ocfs2/inode.c
@@ -1608,6 +1608,10 @@ int ocfs2_validate_inode_block(struct super_block *sb,
 		goto bail;
 	}
 
+	rc = ocfs2_validate_inode_xattr(sb, bh->b_blocknr, di);
+	if (rc)
+		goto bail;
+
 	if (le16_to_cpu(di->i_dyn_features) & OCFS2_INLINE_DATA_FL) {
 		struct ocfs2_inline_data *data = &di->id2.i_data;
 
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index fcddd3c13acd..00900e65634d 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -950,39 +950,105 @@ static int ocfs2_xattr_list_entries(struct inode *inode,
 	return result;
 }
 
-static int ocfs2_xattr_ibody_lookup_header(struct inode *inode,
-					   struct ocfs2_dinode *di,
-					   struct ocfs2_xattr_header **header)
+static int ocfs2_validate_xattr_entries(struct super_block *sb, u64 blkno,
+					struct ocfs2_xattr_header *xh,
+					size_t storage_size,
+					const char *where)
 {
-	u16 xattr_count;
+	u16 xattr_count = le16_to_cpu(xh->xh_count);
 	size_t max_entries;
+	int i;
+
+	if (storage_size < sizeof(*xh))
+		return ocfs2_error(sb,
+				   "Invalid %s in block %llu: storage size %zu is too small\n",
+				   where, (unsigned long long)blkno,
+				   storage_size);
+
+	max_entries = (storage_size - sizeof(*xh)) /
+		      sizeof(struct ocfs2_xattr_entry);
+	if (xattr_count > max_entries) {
+		return ocfs2_error(sb,
+				   "Invalid %s in block %llu: entry count %u exceeds maximum %zu\n",
+				   where, (unsigned long long)blkno,
+				   xattr_count, max_entries);
+	}
+
+	for (i = 0; i < xattr_count; i++) {
+		struct ocfs2_xattr_entry *xe = &xh->xh_entries[i];
+		size_t name_offset = le16_to_cpu(xe->xe_name_offset);
+		size_t value_offset;
+
+		if (name_offset > storage_size ||
+		    xe->xe_name_len > storage_size - name_offset)
+			return ocfs2_error(sb,
+					   "Invalid %s in block %llu: entry %d name is out of bounds\n",
+					   where, (unsigned long long)blkno, i);
+
+		value_offset = name_offset + OCFS2_XATTR_SIZE(xe->xe_name_len);
+		if (value_offset > storage_size)
+			return ocfs2_error(sb,
+					   "Invalid %s in block %llu: entry %d value starts out of bounds\n",
+					   where, (unsigned long long)blkno, i);
+
+		if (ocfs2_xattr_is_local(xe)) {
+			if (le64_to_cpu(xe->xe_value_size) >
+			    storage_size - value_offset)
+				return ocfs2_error(sb,
+						   "Invalid %s in block %llu: entry %d value is out of bounds\n",
+						   where,
+						   (unsigned long long)blkno,
+						   i);
+		} else if (sizeof(struct ocfs2_xattr_value_root) >
+			   storage_size - value_offset) {
+			return ocfs2_error(sb,
+					   "Invalid %s in block %llu: entry %d value root is out of bounds\n",
+					   where, (unsigned long long)blkno, i);
+		}
+	}
+
+	return 0;
+}
+
+static int ocfs2_validate_xattr_ibody_header(struct super_block *sb, u64 blkno,
+					     struct ocfs2_dinode *di,
+					     struct ocfs2_xattr_header **header)
+{
+	struct ocfs2_xattr_header *xh;
 	u16 inline_size = le16_to_cpu(di->i_xattr_inline_size);
 
-	if (inline_size > inode->i_sb->s_blocksize ||
+	if (inline_size > sb->s_blocksize ||
 	    inline_size < sizeof(struct ocfs2_xattr_header)) {
-		ocfs2_error(inode->i_sb,
-			    "Invalid xattr inline size %u in inode %llu\n",
-			    inline_size,
-			    (unsigned long long)OCFS2_I(inode)->ip_blkno);
-		return -EFSCORRUPTED;
+		return ocfs2_error(sb,
+				   "Invalid inode %llu: xattr inline size %u\n",
+				   (unsigned long long)blkno, inline_size);
 	}
 
-	*header = (struct ocfs2_xattr_header *)
-		((void *)di + inode->i_sb->s_blocksize - inline_size);
+	xh = (struct ocfs2_xattr_header *)
+		((void *)di + sb->s_blocksize - inline_size);
+	if (header)
+		*header = xh;
 
-	xattr_count = le16_to_cpu((*header)->xh_count);
-	max_entries = (inline_size - sizeof(struct ocfs2_xattr_header)) /
-		      sizeof(struct ocfs2_xattr_entry);
+	return ocfs2_validate_xattr_entries(sb, blkno, xh, inline_size,
+					    "inline xattr");
+}
 
-	if (xattr_count > max_entries) {
-		ocfs2_error(inode->i_sb,
-			    "xattr entry count %u exceeds maximum %zu in inode %llu\n",
-			    xattr_count, max_entries,
-			    (unsigned long long)OCFS2_I(inode)->ip_blkno);
-		return -EFSCORRUPTED;
-	}
+int ocfs2_validate_inode_xattr(struct super_block *sb, u64 blkno,
+			       struct ocfs2_dinode *di)
+{
+	if (!(le16_to_cpu(di->i_dyn_features) & OCFS2_INLINE_XATTR_FL))
+		return 0;
 
-	return 0;
+	return ocfs2_validate_xattr_ibody_header(sb, blkno, di, NULL);
+}
+
+static int ocfs2_xattr_ibody_lookup_header(struct inode *inode,
+					   struct ocfs2_dinode *di,
+					   struct ocfs2_xattr_header **header)
+{
+	return ocfs2_validate_xattr_ibody_header(inode->i_sb,
+						OCFS2_I(inode)->ip_blkno,
+						di, header);
 }
 
 int ocfs2_has_inline_xattr_value_outside(struct inode *inode,
diff --git a/fs/ocfs2/xattr.h b/fs/ocfs2/xattr.h
index 65e9aa743919..6b7589941315 100644
--- a/fs/ocfs2/xattr.h
+++ b/fs/ocfs2/xattr.h
@@ -43,6 +43,8 @@ int ocfs2_xattr_set_handle(handle_t *, struct inode *, struct buffer_head *,
 			   struct ocfs2_alloc_context *);
 int ocfs2_has_inline_xattr_value_outside(struct inode *inode,
 					 struct ocfs2_dinode *di);
+int ocfs2_validate_inode_xattr(struct super_block *sb, u64 blkno,
+			       struct ocfs2_dinode *di);
 int ocfs2_xattr_remove(struct inode *, struct buffer_head *);
 int ocfs2_init_security_get(struct inode *, struct inode *,
 			    const struct qstr *,
-- 
2.43.0

[PATCH v4 2/2] ocfs2: validate external xattr entries when reading metadata

[Cen Zhang]

ocfs2_validate_xattr_block() checks the xattr block header before the
block reaches higher-level xattr users, but it does not verify that a
non-indexed block's xh_count and entry offsets fit inside the block.
Indexed buckets likewise reach list/get consumers after ECC without an
entry-bounds check.

Move the non-indexed external xattr entry checks into
ocfs2_validate_xattr_block(). Validate indexed bucket entries
immediately after bucket ECC verification in ocfs2_read_xattr_bucket(),
because buckets do not pass through ocfs2_validate_xattr_block().
The bucket entry table is still bounded by the first bucket block; the
name/value offsets are then checked against the bucket block they target.

Reject corrupted external xattr metadata before listxattr() or
getxattr() can walk out-of-range entry arrays or name/value offsets.

Validation reproduced this kernel report:
BUG: KASAN: use-after-free in ocfs2_xattr_list_entries+0xd7/0x190
Read of size 1 at addr ffff88810a654007 by task ocfs2_xattr_lis/630
Call Trace:
  dump_stack_lvl+0x66/0xa0
  print_report+0xce/0x630
  kasan_report+0xe0/0x110
  ocfs2_xattr_list_entries+0xd7/0x190
  ocfs2_listxattr+0x3f6/0x610
  listxattr+0x90/0xe0
  path_listxattrat+0xed/0x220
  do_syscall_64+0x115/0x6a0
  entry_SYSCALL_64_after_hwframe+0x77/0x7f

Fixes: cf1d6c763fbc ("ocfs2: Add extended attribute support")
Fixes: 0c044f0b24b9 ("ocfs2: Add xattr bucket iteration for large numbers of EAs")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 fs/ocfs2/xattr.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 00900e65634d..8147d5c5f293 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -390,6 +390,13 @@ static int ocfs2_init_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
 	return rc;
 }
 
+static int ocfs2_validate_xattr_entries(struct super_block *sb, u64 blkno,
+					struct ocfs2_xattr_header *xh,
+					size_t storage_size,
+					const char *where);
+static int ocfs2_validate_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
+				       u64 blkno);
+
 /* Read the xattr bucket at xb_blkno */
 static int ocfs2_read_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
 				   u64 xb_blkno)
@@ -408,6 +415,8 @@ static int ocfs2_read_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
 		spin_unlock(&OCFS2_SB(bucket->bu_inode->i_sb)->osb_xattr_lock);
 		if (rc)
 			mlog_errno(rc);
+		else
+			rc = ocfs2_validate_xattr_bucket(bucket, xb_blkno);
 	}
 
 	if (rc)
@@ -509,6 +518,16 @@ static int ocfs2_validate_xattr_block(struct super_block *sb,
 				   le32_to_cpu(xb->xb_fs_generation));
 	}
 
+	if (!(le16_to_cpu(xb->xb_flags) & OCFS2_XATTR_INDEXED)) {
+		size_t storage_size = sb->s_blocksize -
+			offsetof(struct ocfs2_xattr_block, xb_attrs.xb_header);
+
+		return ocfs2_validate_xattr_entries(sb, bh->b_blocknr,
+						    &xb->xb_attrs.xb_header,
+						    storage_size,
+						    "xattr block");
+	}
+
 	return 0;
 }
 
@@ -1042,6 +1061,71 @@ int ocfs2_validate_inode_xattr(struct super_block *sb, u64 blkno,
 	return ocfs2_validate_xattr_ibody_header(sb, blkno, di, NULL);
 }
 
+static int ocfs2_validate_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
+				       u64 blkno)
+{
+	struct super_block *sb = bucket->bu_inode->i_sb;
+	struct ocfs2_xattr_header *xh = bucket_xh(bucket);
+	size_t blocksize = sb->s_blocksize;
+	size_t bucket_size = blocksize * bucket->bu_blocks;
+	u16 xattr_count = le16_to_cpu(xh->xh_count);
+	size_t max_entries;
+	int i;
+
+	if (bucket_size < sizeof(*xh))
+		return ocfs2_error(sb,
+				   "Invalid xattr bucket %llu: storage size %zu is too small\n",
+				   (unsigned long long)blkno, bucket_size);
+
+	max_entries = (blocksize - sizeof(*xh)) /
+		      sizeof(struct ocfs2_xattr_entry);
+	if (xattr_count > max_entries)
+		return ocfs2_error(sb,
+				   "Invalid xattr bucket %llu: entry count %u exceeds maximum %zu\n",
+				   (unsigned long long)blkno, xattr_count,
+				   max_entries);
+
+	for (i = 0; i < xattr_count; i++) {
+		struct ocfs2_xattr_entry *xe = &xh->xh_entries[i];
+		size_t name_offset = le16_to_cpu(xe->xe_name_offset);
+		size_t block_off = name_offset >> sb->s_blocksize_bits;
+		size_t block_offset = name_offset % blocksize;
+		size_t value_offset;
+
+		if (name_offset >= bucket_size || block_off >= bucket->bu_blocks)
+			return ocfs2_error(sb,
+					   "Invalid xattr bucket %llu: entry %d name block is out of bounds\n",
+					   (unsigned long long)blkno, i);
+
+		if (xe->xe_name_len > blocksize - block_offset)
+			return ocfs2_error(sb,
+					   "Invalid xattr bucket %llu: entry %d name crosses block boundary\n",
+					   (unsigned long long)blkno, i);
+
+		value_offset = block_offset + OCFS2_XATTR_SIZE(xe->xe_name_len);
+		if (value_offset > blocksize)
+			return ocfs2_error(sb,
+					   "Invalid xattr bucket %llu: entry %d value starts out of bounds\n",
+					   (unsigned long long)blkno, i);
+
+		if (ocfs2_xattr_is_local(xe)) {
+			if (le64_to_cpu(xe->xe_value_size) >
+			    blocksize - value_offset)
+				return ocfs2_error(sb,
+						   "Invalid xattr bucket %llu: entry %d value is out of bounds\n",
+						   (unsigned long long)blkno,
+						   i);
+		} else if (sizeof(struct ocfs2_xattr_value_root) >
+			   blocksize - value_offset) {
+			return ocfs2_error(sb,
+					   "Invalid xattr bucket %llu: entry %d value root is out of bounds\n",
+					   (unsigned long long)blkno, i);
+		}
+	}
+
+	return 0;
+}
+
 static int ocfs2_xattr_ibody_lookup_header(struct inode *inode,
 					   struct ocfs2_dinode *di,
 					   struct ocfs2_xattr_header **header)
-- 
2.43.0

Re: [PATCH v4 1/2] ocfs2: validate inline xattrs during inode block validation

[Joseph Qi]

On 6/22/26 9:03 PM, Cen Zhang wrote:
> ocfs2_validate_inode_block() verifies a dinode before OCFS2 users walk
> metadata from it, but inline xattr metadata is still checked only in
> some consumers. The current ibody helper validates the inline header
> placement and entry count before deriving the header from
> i_xattr_inline_size, but it does not reject entry name/value bounds from
> inode block validation.
> 
> Extend the shared ibody helper with name/value bounds checks and call it
> from ocfs2_validate_inode_block(). Keep the get/list paths using the same
> helper before they derive pointers from i_xattr_inline_size so callers
> with an already-loaded dinode still get the same corruption check.
> 
> Reject corrupted inline xattr metadata before ocfs2_xattr_ibody_get() or
> listxattr() can walk past the inline storage.
> 
> Validation reproduced this kernel report:
> BUG: KASAN: use-after-free in ocfs2_xattr_find_entry+0x5a/0x170
> Read of size 2 at addr ffff8881242a2000 by task python3/529
> Call Trace:
>   dump_stack_lvl+0x66/0xa0
>   print_report+0xce/0x630
>   kasan_report+0xe0/0x110
>   ocfs2_xattr_find_entry+0x5a/0x170
>   ocfs2_xattr_get_nolock+0x20a/0x820
>   ocfs2_xattr_get+0x10c/0x1e0
>   __vfs_getxattr+0xe2/0x130
>   vfs_getxattr+0x185/0x1b0
> 
> Fixes: cf1d6c763fbc ("ocfs2: Add extended attribute support")
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
>  fs/ocfs2/inode.c |   4 ++
>  fs/ocfs2/xattr.c | 112 +++++++++++++++++++++++++++++++++++++----------
>  fs/ocfs2/xattr.h |   2 +
>  3 files changed, 95 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> index 662dbc845b8b..815bf3f659da 100644
> --- a/fs/ocfs2/inode.c
> +++ b/fs/ocfs2/inode.c
> @@ -1608,6 +1608,10 @@ int ocfs2_validate_inode_block(struct super_block *sb,
>  		goto bail;
>  	}
>  
> +	rc = ocfs2_validate_inode_xattr(sb, bh->b_blocknr, di);
> +	if (rc)
> +		goto bail;
> +
>  	if (le16_to_cpu(di->i_dyn_features) & OCFS2_INLINE_DATA_FL) {
>  		struct ocfs2_inline_data *data = &di->id2.i_data;
>  
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index fcddd3c13acd..00900e65634d 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -950,39 +950,105 @@ static int ocfs2_xattr_list_entries(struct inode *inode,
>  	return result;
>  }
>  
> -static int ocfs2_xattr_ibody_lookup_header(struct inode *inode,
> -					   struct ocfs2_dinode *di,
> -					   struct ocfs2_xattr_header **header)
> +static int ocfs2_validate_xattr_entries(struct super_block *sb, u64 blkno,
> +					struct ocfs2_xattr_header *xh,
> +					size_t storage_size,
> +					const char *where)

Seems we can define a ocfs2_xattr_entry_type and pass in, then compute
the size inside ocfs2_validate_xattr_entries().
e.g.

enum ocfs2_xattr_entry_type {
	OCFS2_XATTR_IBODY,
	OCFS2_XATTR_BLOCK,
	OCFS2_XATTR_BUCKET
};

And here 'where' is to identify inline xattr, xattr block or bucket.
Commonly, blkno is already sufficient to uniquely identifies the corrupt
block. So I'd rather drop it for simplification.

>  {
> -	u16 xattr_count;
> +	u16 xattr_count = le16_to_cpu(xh->xh_count);
>  	size_t max_entries;
> +	int i;
> +
> +	if (storage_size < sizeof(*xh))
> +		return ocfs2_error(sb,
> +				   "Invalid %s in block %llu: storage size %zu is too small\n",
> +				   where, (unsigned long long)blkno,
> +				   storage_size);
> +
> +	max_entries = (storage_size - sizeof(*xh)) /
> +		      sizeof(struct ocfs2_xattr_entry);
> +	if (xattr_count > max_entries) {
> +		return ocfs2_error(sb,
> +				   "Invalid %s in block %llu: entry count %u exceeds maximum %zu\n",
> +				   where, (unsigned long long)blkno,
> +				   xattr_count, max_entries);
> +	}
> +
> +	for (i = 0; i < xattr_count; i++) {
> +		struct ocfs2_xattr_entry *xe = &xh->xh_entries[i];
> +		size_t name_offset = le16_to_cpu(xe->xe_name_offset);
> +		size_t value_offset;
> +
> +		if (name_offset > storage_size ||
> +		    xe->xe_name_len > storage_size - name_offset)
> +			return ocfs2_error(sb,
> +					   "Invalid %s in block %llu: entry %d name is out of bounds\n",
> +					   where, (unsigned long long)blkno, i);
> +
> +		value_offset = name_offset + OCFS2_XATTR_SIZE(xe->xe_name_len);
> +		if (value_offset > storage_size)
> +			return ocfs2_error(sb,
> +					   "Invalid %s in block %llu: entry %d value starts out of bounds\n",
> +					   where, (unsigned long long)blkno, i);
> +
> +		if (ocfs2_xattr_is_local(xe)) {
> +			if (le64_to_cpu(xe->xe_value_size) >
> +			    storage_size - value_offset)
> +				return ocfs2_error(sb,
> +						   "Invalid %s in block %llu: entry %d value is out of bounds\n",
> +						   where,
> +						   (unsigned long long)blkno,
> +						   i);
> +		} else if (sizeof(struct ocfs2_xattr_value_root) >
> +			   storage_size - value_offset) {
> +			return ocfs2_error(sb,
> +					   "Invalid %s in block %llu: entry %d value root is out of bounds\n",
> +					   where, (unsigned long long)blkno, i);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int ocfs2_validate_xattr_ibody_header(struct super_block *sb, u64 blkno,
> +					     struct ocfs2_dinode *di,
> +					     struct ocfs2_xattr_header **header)
> +{
> +	struct ocfs2_xattr_header *xh;
>  	u16 inline_size = le16_to_cpu(di->i_xattr_inline_size);
>  
> -	if (inline_size > inode->i_sb->s_blocksize ||
> +	if (inline_size > sb->s_blocksize ||
>  	    inline_size < sizeof(struct ocfs2_xattr_header)) {
> -		ocfs2_error(inode->i_sb,
> -			    "Invalid xattr inline size %u in inode %llu\n",
> -			    inline_size,
> -			    (unsigned long long)OCFS2_I(inode)->ip_blkno);
> -		return -EFSCORRUPTED;
> +		return ocfs2_error(sb,
> +				   "Invalid inode %llu: xattr inline size %u\n",
> +				   (unsigned long long)blkno, inline_size);
>  	}
>  
> -	*header = (struct ocfs2_xattr_header *)
> -		((void *)di + inode->i_sb->s_blocksize - inline_size);
> +	xh = (struct ocfs2_xattr_header *)
> +		((void *)di + sb->s_blocksize - inline_size);
> +	if (header)
> +		*header = xh;
>  
> -	xattr_count = le16_to_cpu((*header)->xh_count);
> -	max_entries = (inline_size - sizeof(struct ocfs2_xattr_header)) /
> -		      sizeof(struct ocfs2_xattr_entry);
> +	return ocfs2_validate_xattr_entries(sb, blkno, xh, inline_size,
> +					    "inline xattr");
> +}
>  
> -	if (xattr_count > max_entries) {
> -		ocfs2_error(inode->i_sb,
> -			    "xattr entry count %u exceeds maximum %zu in inode %llu\n",
> -			    xattr_count, max_entries,
> -			    (unsigned long long)OCFS2_I(inode)->ip_blkno);
> -		return -EFSCORRUPTED;
> -	}
> +int ocfs2_validate_inode_xattr(struct super_block *sb, u64 blkno,
> +			       struct ocfs2_dinode *di)
> +{
> +	if (!(le16_to_cpu(di->i_dyn_features) & OCFS2_INLINE_XATTR_FL))
> +		return 0;
>  
> -	return 0;
> +	return ocfs2_validate_xattr_ibody_header(sb, blkno, di, NULL);
> +}
> +
> +static int ocfs2_xattr_ibody_lookup_header(struct inode *inode,
> +					   struct ocfs2_dinode *di,
> +					   struct ocfs2_xattr_header **header)
> +{
> +	return ocfs2_validate_xattr_ibody_header(inode->i_sb,
> +						OCFS2_I(inode)->ip_blkno,
> +						di, header);
>  }

Since ocfs2_validate_inode_block() already validated at buffer read
time, it seems this per-operation re-check is wasted work.

Thanks,
Joseph

>  
>  int ocfs2_has_inline_xattr_value_outside(struct inode *inode,
> diff --git a/fs/ocfs2/xattr.h b/fs/ocfs2/xattr.h
> index 65e9aa743919..6b7589941315 100644
> --- a/fs/ocfs2/xattr.h
> +++ b/fs/ocfs2/xattr.h
> @@ -43,6 +43,8 @@ int ocfs2_xattr_set_handle(handle_t *, struct inode *, struct buffer_head *,
>  			   struct ocfs2_alloc_context *);
>  int ocfs2_has_inline_xattr_value_outside(struct inode *inode,
>  					 struct ocfs2_dinode *di);
> +int ocfs2_validate_inode_xattr(struct super_block *sb, u64 blkno,
> +			       struct ocfs2_dinode *di);
>  int ocfs2_xattr_remove(struct inode *, struct buffer_head *);
>  int ocfs2_init_security_get(struct inode *, struct inode *,
>  			    const struct qstr *,

Re: [PATCH v4 1/2] ocfs2: validate inline xattrs during inode block validation

[Cen Zhang]

Hi Joseph,

> Seems we can define a ocfs2_xattr_entry_type and pass in, then compute
> the size inside ocfs2_validate_xattr_entries().
> e.g.
>
> enum ocfs2_xattr_entry_type {
>         OCFS2_XATTR_IBODY,
>         OCFS2_XATTR_BLOCK,
>         OCFS2_XATTR_BUCKET
> };
>
> And here 'where' is to identify inline xattr, xattr block or bucket.
> Commonly, blkno is already sufficient to uniquely identifies the corrupt
> block. So I'd rather drop it for simplification.

Thanks for your review and suggestion.  I will rework this for v5.

Best regards,
Cen Zhang

[PATCH v5 0/2] ocfs2: validate xattr entry bounds

[Cen Zhang]

Hi,

This series validates OCFS2 xattr entry name/value bounds when xattr
metadata is read and validated, before getxattr() or listxattr() can
walk out-of-range entry arrays or offsets from corrupted metadata.

Patch 1 validates inline xattrs from ocfs2_validate_inode_block().
Patch 2 reuses the same entry validator for non-indexed xattr blocks
and indexed xattr buckets.

Changes since v4:
  - Replace the descriptive string argument with enum
    ocfs2_xattr_entry_type.
  - Drop the new per-operation full inline-xattr re-check; operation
    paths keep their existing ibody header/count lookup, and full
    entry bounds validation now runs at inode block read time.
  - Simplify corruption messages to rely on the corrupt block number
    instead of also naming the xattr storage kind.

Cen Zhang (2):
  ocfs2: validate inline xattrs during inode block validation
  ocfs2: validate external xattr entries when reading metadata

 fs/ocfs2/inode.c |   4 +
 fs/ocfs2/xattr.c | 194 +++++++++++++++++++++++++++++++++++++++++++----
 fs/ocfs2/xattr.h |   2 +
 3 files changed, 187 insertions(+), 13 deletions(-)

-- 
2.43.0

[PATCH v5 1/2] ocfs2: validate inline xattrs during inode block validation

[Cen Zhang]

ocfs2_validate_inode_block() verifies a dinode before OCFS2 users walk
metadata from it, but inline xattr metadata is still checked only in
operation-specific consumers.  The existing ibody lookup helper validates
inline header placement and entry count, but inode block validation does
not reject entry name/value bounds.

Add a shared xattr entry validator and call it from inode block
validation for inline xattrs.  Keep the operation paths on their existing
header/count lookup checks; the full entry bounds check now runs when the
inode block is validated at read time.

Reject corrupted inline xattr metadata before ocfs2_xattr_ibody_get() or
listxattr() can walk past the inline storage.

Validation reproduced this kernel report:
BUG: KASAN: use-after-free in ocfs2_xattr_find_entry+0x5a/0x170
Read of size 2 at addr ffff8881242a2000 by task python3/529
Call Trace:
  dump_stack_lvl+0x66/0xa0
  print_report+0xce/0x630
  kasan_report+0xe0/0x110
  ocfs2_xattr_find_entry+0x5a/0x170
  ocfs2_xattr_get_nolock+0x20a/0x820
  ocfs2_xattr_get+0x10c/0x1e0
  __vfs_getxattr+0xe2/0x130
  vfs_getxattr+0x185/0x1b0

Fixes: cf1d6c763fbc ("ocfs2: Add extended attribute support")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 fs/ocfs2/inode.c |   4 ++
 fs/ocfs2/xattr.c | 169 +++++++++++++++++++++++++++++++++++++++++++----
 fs/ocfs2/xattr.h |   2 +
 3 files changed, 162 insertions(+), 13 deletions(-)

diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
index 662dbc845b8b..815bf3f659da 100644
--- a/fs/ocfs2/inode.c
+++ b/fs/ocfs2/inode.c
@@ -1608,6 +1608,10 @@ int ocfs2_validate_inode_block(struct super_block *sb,
 		goto bail;
 	}
 
+	rc = ocfs2_validate_inode_xattr(sb, bh->b_blocknr, di);
+	if (rc)
+		goto bail;
+
 	if (le16_to_cpu(di->i_dyn_features) & OCFS2_INLINE_DATA_FL) {
 		struct ocfs2_inline_data *data = &di->id2.i_data;
 
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index fcddd3c13acd..b6f00926849d 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -68,6 +68,12 @@ struct ocfs2_xattr_bucket {
 	int bu_blocks;
 };
 
+enum ocfs2_xattr_entry_type {
+	OCFS2_XATTR_IBODY,
+	OCFS2_XATTR_BLOCK,
+	OCFS2_XATTR_BUCKET,
+};
+
 struct ocfs2_xattr_set_ctxt {
 	handle_t *handle;
 	struct ocfs2_alloc_context *meta_ac;
@@ -950,41 +956,178 @@ static int ocfs2_xattr_list_entries(struct inode *inode,
 	return result;
 }
 
-static int ocfs2_xattr_ibody_lookup_header(struct inode *inode,
-					   struct ocfs2_dinode *di,
-					   struct ocfs2_xattr_header **header)
+static int ocfs2_validate_xattr_entries(struct super_block *sb, u64 blkno,
+					struct ocfs2_xattr_header *xh,
+					enum ocfs2_xattr_entry_type type,
+					size_t storage_size)
+{
+	u16 xattr_count = le16_to_cpu(xh->xh_count);
+	size_t entry_storage_size = storage_size;
+	size_t max_entries;
+	int i;
+
+	switch (type) {
+	case OCFS2_XATTR_IBODY:
+		break;
+	case OCFS2_XATTR_BLOCK:
+		storage_size = sb->s_blocksize -
+			offsetof(struct ocfs2_xattr_block, xb_attrs.xb_header);
+		entry_storage_size = storage_size;
+		break;
+	case OCFS2_XATTR_BUCKET:
+		entry_storage_size = sb->s_blocksize;
+		break;
+	}
+
+	if (storage_size < sizeof(*xh))
+		return ocfs2_error(sb,
+				   "Invalid xattr in block %llu: storage size %zu is too small\n",
+				   (unsigned long long)blkno, storage_size);
+
+	if (entry_storage_size < sizeof(*xh))
+		return ocfs2_error(sb,
+				   "Invalid xattr in block %llu: entry storage size %zu is too small\n",
+				   (unsigned long long)blkno,
+				   entry_storage_size);
+
+	max_entries = (entry_storage_size - sizeof(*xh)) /
+		      sizeof(struct ocfs2_xattr_entry);
+
+	if (xattr_count > max_entries)
+		return ocfs2_error(sb,
+				   "Invalid xattr in block %llu: entry count %u exceeds maximum %zu\n",
+				   (unsigned long long)blkno,
+				   xattr_count, max_entries);
+
+	for (i = 0; i < xattr_count; i++) {
+		struct ocfs2_xattr_entry *xe = &xh->xh_entries[i];
+		size_t name_offset = le16_to_cpu(xe->xe_name_offset);
+		size_t value_offset;
+		size_t value_limit = storage_size;
+
+		if (type == OCFS2_XATTR_BUCKET) {
+			size_t block_offset;
+
+			if (name_offset >= storage_size)
+				return ocfs2_error(sb,
+						   "Invalid xattr in block %llu: entry %d name is out of bounds\n",
+						   (unsigned long long)blkno,
+						   i);
+
+			block_offset = name_offset % sb->s_blocksize;
+			if (xe->xe_name_len > sb->s_blocksize - block_offset)
+				return ocfs2_error(sb,
+						   "Invalid xattr in block %llu: entry %d name crosses block boundary\n",
+						   (unsigned long long)blkno,
+						   i);
+
+			value_offset = block_offset +
+				OCFS2_XATTR_SIZE(xe->xe_name_len);
+			value_limit = sb->s_blocksize;
+		} else {
+			if (name_offset > storage_size ||
+			    xe->xe_name_len > storage_size - name_offset)
+				return ocfs2_error(sb,
+						   "Invalid xattr in block %llu: entry %d name is out of bounds\n",
+						   (unsigned long long)blkno,
+						   i);
+
+			value_offset = name_offset +
+				OCFS2_XATTR_SIZE(xe->xe_name_len);
+		}
+
+		if (value_offset > value_limit)
+			return ocfs2_error(sb,
+					   "Invalid xattr in block %llu: entry %d value starts out of bounds\n",
+					   (unsigned long long)blkno, i);
+
+		if (ocfs2_xattr_is_local(xe)) {
+			if (le64_to_cpu(xe->xe_value_size) >
+			    value_limit - value_offset)
+				return ocfs2_error(sb,
+						   "Invalid xattr in block %llu: entry %d value is out of bounds\n",
+						   (unsigned long long)blkno,
+						   i);
+		} else if (sizeof(struct ocfs2_xattr_value_root) >
+			   value_limit - value_offset) {
+			return ocfs2_error(sb,
+					   "Invalid xattr in block %llu: entry %d value root is out of bounds\n",
+					   (unsigned long long)blkno, i);
+		}
+	}
+
+	return 0;
+}
+
+static int ocfs2_xattr_ibody_lookup_header_raw(struct super_block *sb,
+					       u64 blkno,
+					       struct ocfs2_dinode *di,
+					       struct ocfs2_xattr_header **header,
+					       u16 *inline_size_ret)
 {
+	struct ocfs2_xattr_header *xh;
 	u16 xattr_count;
 	size_t max_entries;
 	u16 inline_size = le16_to_cpu(di->i_xattr_inline_size);
 
-	if (inline_size > inode->i_sb->s_blocksize ||
+	if (inline_size > sb->s_blocksize ||
 	    inline_size < sizeof(struct ocfs2_xattr_header)) {
-		ocfs2_error(inode->i_sb,
-			    "Invalid xattr inline size %u in inode %llu\n",
-			    inline_size,
-			    (unsigned long long)OCFS2_I(inode)->ip_blkno);
+		ocfs2_error(sb,
+			    "Invalid inode %llu: xattr inline size %u\n",
+			    (unsigned long long)blkno, inline_size);
 		return -EFSCORRUPTED;
 	}
 
-	*header = (struct ocfs2_xattr_header *)
-		((void *)di + inode->i_sb->s_blocksize - inline_size);
+	xh = (struct ocfs2_xattr_header *)
+		((void *)di + sb->s_blocksize - inline_size);
 
-	xattr_count = le16_to_cpu((*header)->xh_count);
+	xattr_count = le16_to_cpu(xh->xh_count);
 	max_entries = (inline_size - sizeof(struct ocfs2_xattr_header)) /
 		      sizeof(struct ocfs2_xattr_entry);
 
 	if (xattr_count > max_entries) {
-		ocfs2_error(inode->i_sb,
+		ocfs2_error(sb,
 			    "xattr entry count %u exceeds maximum %zu in inode %llu\n",
 			    xattr_count, max_entries,
-			    (unsigned long long)OCFS2_I(inode)->ip_blkno);
+			    (unsigned long long)blkno);
 		return -EFSCORRUPTED;
 	}
 
+	*header = xh;
+	if (inline_size_ret)
+		*inline_size_ret = inline_size;
+
 	return 0;
 }
 
+int ocfs2_validate_inode_xattr(struct super_block *sb, u64 blkno,
+			       struct ocfs2_dinode *di)
+{
+	struct ocfs2_xattr_header *xh;
+	u16 inline_size;
+	int ret;
+
+	if (!(le16_to_cpu(di->i_dyn_features) & OCFS2_INLINE_XATTR_FL))
+		return 0;
+
+	ret = ocfs2_xattr_ibody_lookup_header_raw(sb, blkno, di, &xh,
+						  &inline_size);
+	if (ret)
+		return ret;
+
+	return ocfs2_validate_xattr_entries(sb, blkno, xh, OCFS2_XATTR_IBODY,
+					    inline_size);
+}
+
+static int ocfs2_xattr_ibody_lookup_header(struct inode *inode,
+					   struct ocfs2_dinode *di,
+					   struct ocfs2_xattr_header **header)
+{
+	return ocfs2_xattr_ibody_lookup_header_raw(inode->i_sb,
+						   OCFS2_I(inode)->ip_blkno,
+						   di, header, NULL);
+}
+
 int ocfs2_has_inline_xattr_value_outside(struct inode *inode,
 					 struct ocfs2_dinode *di)
 {
diff --git a/fs/ocfs2/xattr.h b/fs/ocfs2/xattr.h
index 65e9aa743919..6b7589941315 100644
--- a/fs/ocfs2/xattr.h
+++ b/fs/ocfs2/xattr.h
@@ -43,6 +43,8 @@ int ocfs2_xattr_set_handle(handle_t *, struct inode *, struct buffer_head *,
 			   struct ocfs2_alloc_context *);
 int ocfs2_has_inline_xattr_value_outside(struct inode *inode,
 					 struct ocfs2_dinode *di);
+int ocfs2_validate_inode_xattr(struct super_block *sb, u64 blkno,
+			       struct ocfs2_dinode *di);
 int ocfs2_xattr_remove(struct inode *, struct buffer_head *);
 int ocfs2_init_security_get(struct inode *, struct inode *,
 			    const struct qstr *,
-- 
2.43.0

[PATCH v5 2/2] ocfs2: validate external xattr entries when reading metadata

[Cen Zhang]

ocfs2_validate_xattr_block() checks the xattr block header before the
block reaches higher-level xattr users, but it does not verify that a
non-indexed block's xh_count and entry offsets fit inside the block.
Indexed buckets likewise reach list/get consumers after ECC without an
entry-bounds check.

Reuse the xattr entry validator for non-indexed external xattr blocks
and indexed buckets at metadata read time.  The enum entry type selects
the storage geometry: non-indexed blocks use the xattr block payload,
while buckets keep the entry table bounded by the first bucket block and
check name/value offsets against the bucket block they target.

Reject corrupted external xattr metadata before listxattr() or
getxattr() can walk out-of-range entry arrays or name/value offsets.

Validation reproduced this kernel report:
BUG: KASAN: use-after-free in ocfs2_xattr_list_entries+0xd7/0x190
Read of size 1 at addr ffff88810a654007 by task ocfs2_xattr_lis/630
Call Trace:
  dump_stack_lvl+0x66/0xa0
  print_report+0xce/0x630
  kasan_report+0xe0/0x110
  ocfs2_xattr_list_entries+0xd7/0x190
  ocfs2_listxattr+0x3f6/0x610
  listxattr+0x90/0xe0
  path_listxattrat+0xed/0x220
  do_syscall_64+0x115/0x6a0
  entry_SYSCALL_64_after_hwframe+0x77/0x7f

Fixes: cf1d6c763fbc ("ocfs2: Add extended attribute support")
Fixes: 0c044f0b24b9 ("ocfs2: Add xattr bucket iteration for large numbers of EAs")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 fs/ocfs2/xattr.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index b6f00926849d..cb0775d526b8 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -396,6 +396,13 @@ static int ocfs2_init_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
 	return rc;
 }
 
+static int ocfs2_validate_xattr_entries(struct super_block *sb, u64 blkno,
+					struct ocfs2_xattr_header *xh,
+					enum ocfs2_xattr_entry_type type,
+					size_t storage_size);
+static int ocfs2_validate_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
+				       u64 blkno);
+
 /* Read the xattr bucket at xb_blkno */
 static int ocfs2_read_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
 				   u64 xb_blkno)
@@ -414,6 +421,8 @@ static int ocfs2_read_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
 		spin_unlock(&OCFS2_SB(bucket->bu_inode->i_sb)->osb_xattr_lock);
 		if (rc)
 			mlog_errno(rc);
+		else
+			rc = ocfs2_validate_xattr_bucket(bucket, xb_blkno);
 	}
 
 	if (rc)
@@ -515,6 +524,11 @@ static int ocfs2_validate_xattr_block(struct super_block *sb,
 				   le32_to_cpu(xb->xb_fs_generation));
 	}
 
+	if (!(le16_to_cpu(xb->xb_flags) & OCFS2_XATTR_INDEXED))
+		return ocfs2_validate_xattr_entries(sb, bh->b_blocknr,
+						    &xb->xb_attrs.xb_header,
+						    OCFS2_XATTR_BLOCK, 0);
+
 	return 0;
 }
 
@@ -1119,6 +1133,17 @@ int ocfs2_validate_inode_xattr(struct super_block *sb, u64 blkno,
 					    inline_size);
 }
 
+static int ocfs2_validate_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
+				       u64 blkno)
+{
+	struct super_block *sb = bucket->bu_inode->i_sb;
+
+	return ocfs2_validate_xattr_entries(sb, blkno, bucket_xh(bucket),
+					    OCFS2_XATTR_BUCKET,
+					    sb->s_blocksize *
+					    bucket->bu_blocks);
+}
+
 static int ocfs2_xattr_ibody_lookup_header(struct inode *inode,
 					   struct ocfs2_dinode *di,
 					   struct ocfs2_xattr_header **header)
-- 
2.43.0