[PATCH] ocfs2: validate the xattr tree root of an indexed xattr block

[PATCH] ocfs2: validate the xattr tree root of an indexed xattr block

[Joseph Qi]

ocfs2_validate_xattr_block() checks an xattr block's signature, blkno,
generation and suballocator fields, and for a non-indexed block the entry
array it carries.  An indexed block is passed straight through, so the
extent list in its xattr tree root is never looked at.

Both fields that describe the list are bounded.  The two places that
create a root set l_count from ocfs2_xattr_recs_per_xb() and nothing
rewrites it, and the insert path asserts in ocfs2_rotate_leaf() that
l_next_free_rec stays within l_count.  Check both when the block is
first read, so a root that disagrees with the block size, or claims more
records than it has room for, is caught before anything indexes into
l_recs[].

An empty root is deliberately not rejected.  Emptying the tree leaves it
that way: ocfs2_rm_xattr_cluster() removes one extent per transaction,
and the last one ends in ocfs2_truncate_rec(), whose closing
ocfs2_rotate_tree_left() reaches ocfs2_remove_empty_extent() on the
depth-zero root and takes l_next_free_rec to zero.  That transaction
commits before ocfs2_xattr_free_block() frees the block, and before
ocfs2_xattr_remove() clears i_xattr_loc, in two later transactions, so a
crash in between leaves an orphan inode pointing at an xattr block whose
root has no extents.  Both readers already cope with it:
__ocfs2_xattr_index_block_find() answers -ENODATA, and
ocfs2_iterate_xattr_index_block() returns without iterating.

Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/ocfs2/xattr.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 18d920b30602..b2bd06083a14 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -491,6 +491,7 @@ static int ocfs2_validate_xattr_block(struct super_block *sb,
 				      struct buffer_head *bh)
 {
 	int rc;
+	struct ocfs2_extent_list *el;
 	struct ocfs2_xattr_block *xb =
 		(struct ocfs2_xattr_block *)bh->b_data;
 
@@ -573,6 +574,30 @@ static int ocfs2_validate_xattr_block(struct super_block *sb,
 							 region_offset);
 	}
 
+	/*
+	 * An indexed xattr block carries the root of the xattr tree.  Its
+	 * record count is fixed by the block size and l_next_free_rec never
+	 * exceeds it.  A zero l_next_free_rec is valid: emptying the tree on
+	 * the inode deletion path leaves the root with no extents.
+	 */
+	el = &xb->xb_attrs.xb_root.xt_list;
+
+	if (le16_to_cpu(el->l_count) != ocfs2_xattr_recs_per_xb(sb)) {
+		return ocfs2_error(sb,
+				   "Xattr tree root #%llu has invalid l_count %u (expected %u)\n",
+				   (unsigned long long)bh->b_blocknr,
+				   le16_to_cpu(el->l_count),
+				   ocfs2_xattr_recs_per_xb(sb));
+	}
+
+	if (le16_to_cpu(el->l_next_free_rec) > le16_to_cpu(el->l_count)) {
+		return ocfs2_error(sb,
+				   "Xattr tree root #%llu has l_next_free_rec %u past the end of its list (l_count %u)\n",
+				   (unsigned long long)bh->b_blocknr,
+				   le16_to_cpu(el->l_next_free_rec),
+				   le16_to_cpu(el->l_count));
+	}
+
 	return 0;
 }
 
-- 
2.39.3

Re: [PATCH] ocfs2: validate the xattr tree root of an indexed xattr block

[Heming Zhao]

On Wed, Oct 14, 2026 at 06:58:56PM +0800, Joseph Qi wrote:
> ocfs2_validate_xattr_block() checks an xattr block's signature, blkno,
> generation and suballocator fields, and for a non-indexed block the entry
> array it carries.  An indexed block is passed straight through, so the
> extent list in its xattr tree root is never looked at.
> 
> Both fields that describe the list are bounded.  The two places that
> create a root set l_count from ocfs2_xattr_recs_per_xb() and nothing
> rewrites it, and the insert path asserts in ocfs2_rotate_leaf() that
> l_next_free_rec stays within l_count.  Check both when the block is
> first read, so a root that disagrees with the block size, or claims more
> records than it has room for, is caught before anything indexes into
> l_recs[].
> 
> An empty root is deliberately not rejected.  Emptying the tree leaves it
> that way: ocfs2_rm_xattr_cluster() removes one extent per transaction,
> and the last one ends in ocfs2_truncate_rec(), whose closing
> ocfs2_rotate_tree_left() reaches ocfs2_remove_empty_extent() on the
> depth-zero root and takes l_next_free_rec to zero.  That transaction
> commits before ocfs2_xattr_free_block() frees the block, and before
> ocfs2_xattr_remove() clears i_xattr_loc, in two later transactions, so a
> crash in between leaves an orphan inode pointing at an xattr block whose
> root has no extents.  Both readers already cope with it:
> __ocfs2_xattr_index_block_find() answers -ENODATA, and
> ocfs2_iterate_xattr_index_block() returns without iterating.
> 
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>

LGTM.
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
>  fs/ocfs2/xattr.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index 18d920b30602..b2bd06083a14 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -491,6 +491,7 @@ static int ocfs2_validate_xattr_block(struct super_block *sb,
>  				      struct buffer_head *bh)
>  {
>  	int rc;
> +	struct ocfs2_extent_list *el;
>  	struct ocfs2_xattr_block *xb =
>  		(struct ocfs2_xattr_block *)bh->b_data;
>  
> @@ -573,6 +574,30 @@ static int ocfs2_validate_xattr_block(struct super_block *sb,
>  							 region_offset);
>  	}
>  
> +	/*
> +	 * An indexed xattr block carries the root of the xattr tree.  Its
> +	 * record count is fixed by the block size and l_next_free_rec never
> +	 * exceeds it.  A zero l_next_free_rec is valid: emptying the tree on
> +	 * the inode deletion path leaves the root with no extents.
> +	 */
> +	el = &xb->xb_attrs.xb_root.xt_list;
> +
> +	if (le16_to_cpu(el->l_count) != ocfs2_xattr_recs_per_xb(sb)) {
> +		return ocfs2_error(sb,
> +				   "Xattr tree root #%llu has invalid l_count %u (expected %u)\n",
> +				   (unsigned long long)bh->b_blocknr,
> +				   le16_to_cpu(el->l_count),
> +				   ocfs2_xattr_recs_per_xb(sb));
> +	}
> +
> +	if (le16_to_cpu(el->l_next_free_rec) > le16_to_cpu(el->l_count)) {
> +		return ocfs2_error(sb,
> +				   "Xattr tree root #%llu has l_next_free_rec %u past the end of its list (l_count %u)\n",
> +				   (unsigned long long)bh->b_blocknr,
> +				   le16_to_cpu(el->l_next_free_rec),
> +				   le16_to_cpu(el->l_count));
> +	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.39.3
>