mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v7 0/2] ocfs2: validate xattr entry bounds
@ 2026-07-05  2:53 Cen Zhang
  2026-07-05  2:53 ` [PATCH v7 1/2] ocfs2: validate inline xattrs during inode block validation Cen Zhang
  2026-07-05  2:53 ` [PATCH v7 2/2] ocfs2: validate external xattr entries when reading metadata Cen Zhang
  0 siblings, 2 replies; 5+ messages in thread
From: Cen Zhang @ 2026-07-05  2:53 UTC (permalink / raw)
  To: Joseph Qi, Mark Fasheh, Joel Becker; +Cc: ocfs2-devel, linux-kernel, zzzccc427

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 validates non-indexed xattr blocks from ocfs2_validate_xattr_block()
and indexed xattr buckets after bucket ECC validation.

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 | 215 ++++++++++++++++++++++++++++++++++++++++++++---
 fs/ocfs2/xattr.h |   2 +
 3 files changed, 208 insertions(+), 13 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v7 1/2] ocfs2: validate inline xattrs during inode block validation
  2026-07-05  2:53 [PATCH v7 0/2] ocfs2: validate xattr entry bounds Cen Zhang
@ 2026-07-05  2:53 ` Cen Zhang
  2026-07-06  6:24   ` Joseph Qi
  2026-07-05  2:53 ` [PATCH v7 2/2] ocfs2: validate external xattr entries when reading metadata Cen Zhang
  1 sibling, 1 reply; 5+ messages in thread
From: Cen Zhang @ 2026-07-05  2:53 UTC (permalink / raw)
  To: Joseph Qi, Mark Fasheh, Joel Becker; +Cc: ocfs2-devel, linux-kernel, zzzccc427

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 flat 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 | 118 +++++++++++++++++++++++++++++++++++++++++------
 fs/ocfs2/xattr.h |   2 +
 3 files changed, 111 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..ca76441625db 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -950,41 +950,133 @@ 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_flat(struct super_block *sb, u64 blkno,
+					     struct ocfs2_xattr_header *xh,
+					     size_t region_size)
+{
+	u16 xattr_count = le16_to_cpu(xh->xh_count);
+	size_t entries_limit = region_size;
+	size_t nv_limit = region_size;
+	size_t max_entries;
+	int i;
+
+	if (region_size < sizeof(*xh))
+		return ocfs2_error(sb,
+				   "Invalid xattr in block %llu: region size %zu is too small\n",
+				   (unsigned long long)blkno, region_size);
+
+	max_entries = (entries_limit - 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;
+
+		if (name_offset > nv_limit ||
+		    xe->xe_name_len > nv_limit - 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 > nv_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) >
+			    nv_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) >
+			   nv_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_flat(sb, blkno, xh, 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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v7 2/2] ocfs2: validate external xattr entries when reading metadata
  2026-07-05  2:53 [PATCH v7 0/2] ocfs2: validate xattr entry bounds Cen Zhang
  2026-07-05  2:53 ` [PATCH v7 1/2] ocfs2: validate inline xattrs during inode block validation Cen Zhang
@ 2026-07-05  2:53 ` Cen Zhang
  2026-07-06  6:24   ` Joseph Qi
  1 sibling, 1 reply; 5+ messages in thread
From: Cen Zhang @ 2026-07-05  2:53 UTC (permalink / raw)
  To: Joseph Qi, Mark Fasheh, Joel Becker; +Cc: ocfs2-devel, linux-kernel, zzzccc427

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.

Use the flat xattr entry validator for non-indexed external xattr
blocks, and use a bucket-specific validator for indexed buckets at
metadata read time. The bucket validator keeps the entry array bounded
by the first bucket block while checking 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 | 97 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index ca76441625db..d23364c30e1a 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -390,6 +390,12 @@ static int ocfs2_init_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
 	return rc;
 }
 
+static int ocfs2_validate_xattr_entries_flat(struct super_block *sb, u64 blkno,
+					     struct ocfs2_xattr_header *xh,
+					     size_t region_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)
@@ -408,6 +414,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 +517,22 @@ 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 region_offset =
+			offsetof(struct ocfs2_xattr_block, xb_attrs.xb_header);
+
+		if (bh->b_size < region_offset)
+			return ocfs2_error(sb,
+					   "Invalid xattr block %llu: block size %zu is too small\n",
+					   (unsigned long long)bh->b_blocknr,
+					   bh->b_size);
+
+		return ocfs2_validate_xattr_entries_flat(sb, bh->b_blocknr,
+							 &xb->xb_attrs.xb_header,
+							 bh->b_size -
+							 region_offset);
+	}
+
 	return 0;
 }
 
@@ -1068,6 +1092,79 @@ int ocfs2_validate_inode_xattr(struct super_block *sb, u64 blkno,
 	return ocfs2_validate_xattr_entries_flat(sb, blkno, xh, inline_size);
 }
 
+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);
+	u16 xattr_count = le16_to_cpu(xh->xh_count);
+	size_t region_size = (size_t)sb->s_blocksize * bucket->bu_blocks;
+	size_t entries_limit = sb->s_blocksize;
+	size_t nv_limit = sb->s_blocksize;
+	size_t max_entries;
+	int i;
+
+	if (region_size < sizeof(*xh))
+		return ocfs2_error(sb,
+				   "Invalid xattr bucket %llu: region size %zu is too small\n",
+				   (unsigned long long)blkno, region_size);
+
+	if (entries_limit < sizeof(*xh))
+		return ocfs2_error(sb,
+				   "Invalid xattr bucket %llu: entries limit %zu is too small\n",
+				   (unsigned long long)blkno,
+				   entries_limit);
+
+	max_entries = (entries_limit - 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 % nv_limit;
+		size_t value_offset;
+
+		if (name_offset >= region_size || block_off >= bucket->bu_blocks)
+			return ocfs2_error(sb,
+					   "Invalid xattr bucket %llu: entry %d name is out of bounds\n",
+					   (unsigned long long)blkno, i);
+
+		if (xe->xe_name_len > nv_limit - 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 > nv_limit)
+			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) >
+			    nv_limit - 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) >
+			   nv_limit - 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v7 1/2] ocfs2: validate inline xattrs during inode block validation
  2026-07-05  2:53 ` [PATCH v7 1/2] ocfs2: validate inline xattrs during inode block validation Cen Zhang
@ 2026-07-06  6:24   ` Joseph Qi
  0 siblings, 0 replies; 5+ messages in thread
From: Joseph Qi @ 2026-07-06  6:24 UTC (permalink / raw)
  To: Cen Zhang, akpm; +Cc: ocfs2-devel, linux-kernel, Mark Fasheh, Joel Becker



On 7/5/26 10:53 AM, 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
> 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 flat 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>

Looks fine.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

> ---
>  fs/ocfs2/inode.c |   4 ++
>  fs/ocfs2/xattr.c | 118 +++++++++++++++++++++++++++++++++++++++++------
>  fs/ocfs2/xattr.h |   2 +
>  3 files changed, 111 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..ca76441625db 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -950,41 +950,133 @@ 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_flat(struct super_block *sb, u64 blkno,
> +					     struct ocfs2_xattr_header *xh,
> +					     size_t region_size)
> +{
> +	u16 xattr_count = le16_to_cpu(xh->xh_count);
> +	size_t entries_limit = region_size;
> +	size_t nv_limit = region_size;
> +	size_t max_entries;
> +	int i;
> +
> +	if (region_size < sizeof(*xh))
> +		return ocfs2_error(sb,
> +				   "Invalid xattr in block %llu: region size %zu is too small\n",
> +				   (unsigned long long)blkno, region_size);
> +
> +	max_entries = (entries_limit - 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;
> +
> +		if (name_offset > nv_limit ||
> +		    xe->xe_name_len > nv_limit - 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 > nv_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) >
> +			    nv_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) >
> +			   nv_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_flat(sb, blkno, xh, 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 *,


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v7 2/2] ocfs2: validate external xattr entries when reading metadata
  2026-07-05  2:53 ` [PATCH v7 2/2] ocfs2: validate external xattr entries when reading metadata Cen Zhang
@ 2026-07-06  6:24   ` Joseph Qi
  0 siblings, 0 replies; 5+ messages in thread
From: Joseph Qi @ 2026-07-06  6:24 UTC (permalink / raw)
  To: Cen Zhang, akpm; +Cc: ocfs2-devel, linux-kernel, Mark Fasheh, Joel Becker



On 7/5/26 10:53 AM, Cen Zhang wrote:
> 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.
> 
> Use the flat xattr entry validator for non-indexed external xattr
> blocks, and use a bucket-specific validator for indexed buckets at
> metadata read time. The bucket validator keeps the entry array bounded
> by the first bucket block while checking 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>

Looks fine.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
>  fs/ocfs2/xattr.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 97 insertions(+)
> 
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index ca76441625db..d23364c30e1a 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -390,6 +390,12 @@ static int ocfs2_init_xattr_bucket(struct ocfs2_xattr_bucket *bucket,
>  	return rc;
>  }
>  
> +static int ocfs2_validate_xattr_entries_flat(struct super_block *sb, u64 blkno,
> +					     struct ocfs2_xattr_header *xh,
> +					     size_t region_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)
> @@ -408,6 +414,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 +517,22 @@ 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 region_offset =
> +			offsetof(struct ocfs2_xattr_block, xb_attrs.xb_header);
> +
> +		if (bh->b_size < region_offset)
> +			return ocfs2_error(sb,
> +					   "Invalid xattr block %llu: block size %zu is too small\n",
> +					   (unsigned long long)bh->b_blocknr,
> +					   bh->b_size);
> +
> +		return ocfs2_validate_xattr_entries_flat(sb, bh->b_blocknr,
> +							 &xb->xb_attrs.xb_header,
> +							 bh->b_size -
> +							 region_offset);
> +	}
> +
>  	return 0;
>  }
>  
> @@ -1068,6 +1092,79 @@ int ocfs2_validate_inode_xattr(struct super_block *sb, u64 blkno,
>  	return ocfs2_validate_xattr_entries_flat(sb, blkno, xh, inline_size);
>  }
>  
> +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);
> +	u16 xattr_count = le16_to_cpu(xh->xh_count);
> +	size_t region_size = (size_t)sb->s_blocksize * bucket->bu_blocks;
> +	size_t entries_limit = sb->s_blocksize;
> +	size_t nv_limit = sb->s_blocksize;
> +	size_t max_entries;
> +	int i;
> +
> +	if (region_size < sizeof(*xh))
> +		return ocfs2_error(sb,
> +				   "Invalid xattr bucket %llu: region size %zu is too small\n",
> +				   (unsigned long long)blkno, region_size);
> +
> +	if (entries_limit < sizeof(*xh))
> +		return ocfs2_error(sb,
> +				   "Invalid xattr bucket %llu: entries limit %zu is too small\n",
> +				   (unsigned long long)blkno,
> +				   entries_limit);
> +
> +	max_entries = (entries_limit - 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 % nv_limit;
> +		size_t value_offset;
> +
> +		if (name_offset >= region_size || block_off >= bucket->bu_blocks)
> +			return ocfs2_error(sb,
> +					   "Invalid xattr bucket %llu: entry %d name is out of bounds\n",
> +					   (unsigned long long)blkno, i);
> +
> +		if (xe->xe_name_len > nv_limit - 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 > nv_limit)
> +			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) >
> +			    nv_limit - 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) >
> +			   nv_limit - 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)


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-06  6:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-05  2:53 [PATCH v7 0/2] ocfs2: validate xattr entry bounds Cen Zhang
2026-07-05  2:53 ` [PATCH v7 1/2] ocfs2: validate inline xattrs during inode block validation Cen Zhang
2026-07-06  6:24   ` Joseph Qi
2026-07-05  2:53 ` [PATCH v7 2/2] ocfs2: validate external xattr entries when reading metadata Cen Zhang
2026-07-06  6:24   ` Joseph Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome