[PATCH v2 RESEND] xfs: bound da-node entry count against the correct geometry

[PATCH v2 RESEND] xfs: bound da-node entry count against the correct geometry

[Aldo Ariel Panzardo]

xfs_da3_node_verify() bounds the node entry count against the larger of
the directory and attribute geometries because, as a buffer verifier, it
cannot tell whether the block belongs to the directory or the attribute
tree. When the directory block size exceeds the fs block size (e.g.
mkfs.xfs -n size=64k -b size=4k), an attribute node buffer is a single fs
block that holds only m_attr_geo->node_ents entries, yet a crafted attr
node may claim a count up to m_dir_geo->node_ents and still pass the
verifier.

xfs_da3_node_lookup_int() then indexes btree[] up to that count during
its binary search -- an out-of-bounds read via getxattr/listxattr on a
mounted crafted image.

The buffer verifier is the wrong place to tighten this: it has no fork
context, and the transaction-less read path used by getxattr does not run
xfs_da3_node_set_type() either. xfs_da3_node_lookup_int(), on the other
hand, always runs on that path and holds args->geo, the geometry of the
fork actually being searched. Bound the entry count against
args->geo->node_ents there, before walking the entries.

Fixes: 7ab610f9e0f1 ("xfs: move node entry counts to xfs_da_geometry")
Cc: <stable@vger.kernel.org>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
v2: reworked per Darrick's review.  Do not infer dir-vs-attr from the
    buffer size in the verifier; instead bound the entry count in
    xfs_da3_node_lookup_int() against args->geo->node_ents, the
    geometry of the fork being searched.  cc stable.

 fs/xfs/libxfs/xfs_da_btree.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 9debb95d86fa..95ea3737eb33 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -1787,6 +1787,20 @@ xfs_da3_node_lookup_int(
 		} else
 			expected_level--;
 
+		/*
+		 * The node verifier cannot tell whether this block belongs to
+		 * the directory or the attribute tree, so it only bounds the
+		 * entry count against the larger of the two geometries.  Here
+		 * args->geo is the geometry of the fork we are actually
+		 * searching, so reject a count that would walk btree[] off the
+		 * end of this node buffer.
+		 */
+		if (nodehdr.count > args->geo->node_ents) {
+			xfs_buf_mark_corrupt(blk->bp);
+			xfs_da_mark_sick(args);
+			return -EFSCORRUPTED;
+		}
+
 		max = nodehdr.count;
 		blk->hashval = be32_to_cpu(btree[max - 1].hashval);
 
-- 
2.53.0

Re: [PATCH v2 RESEND] xfs: bound da-node entry count against the correct geometry

[Darrick J. Wong]

On Wed, Sep 23, 2026 at 08:57:54AM -0300, Aldo Ariel Panzardo wrote:
> xfs_da3_node_verify() bounds the node entry count against the larger of
> the directory and attribute geometries because, as a buffer verifier, it
> cannot tell whether the block belongs to the directory or the attribute
> tree. When the directory block size exceeds the fs block size (e.g.
> mkfs.xfs -n size=64k -b size=4k), an attribute node buffer is a single fs
> block that holds only m_attr_geo->node_ents entries, yet a crafted attr
> node may claim a count up to m_dir_geo->node_ents and still pass the
> verifier.
> 
> xfs_da3_node_lookup_int() then indexes btree[] up to that count during
> its binary search -- an out-of-bounds read via getxattr/listxattr on a
> mounted crafted image.
> 
> The buffer verifier is the wrong place to tighten this: it has no fork
> context, and the transaction-less read path used by getxattr does not run
> xfs_da3_node_set_type() either. xfs_da3_node_lookup_int(), on the other
> hand, always runs on that path and holds args->geo, the geometry of the
> fork actually being searched. Bound the entry count against
> args->geo->node_ents there, before walking the entries.
> 
> Fixes: 7ab610f9e0f1 ("xfs: move node entry counts to xfs_da_geometry")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

I must've missed this, so thanks for reposting :)
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
> v2: reworked per Darrick's review.  Do not infer dir-vs-attr from the
>     buffer size in the verifier; instead bound the entry count in
>     xfs_da3_node_lookup_int() against args->geo->node_ents, the
>     geometry of the fork being searched.  cc stable.
> 
>  fs/xfs/libxfs/xfs_da_btree.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 9debb95d86fa..95ea3737eb33 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -1787,6 +1787,20 @@ xfs_da3_node_lookup_int(
>  		} else
>  			expected_level--;
>  
> +		/*
> +		 * The node verifier cannot tell whether this block belongs to
> +		 * the directory or the attribute tree, so it only bounds the
> +		 * entry count against the larger of the two geometries.  Here
> +		 * args->geo is the geometry of the fork we are actually
> +		 * searching, so reject a count that would walk btree[] off the
> +		 * end of this node buffer.
> +		 */
> +		if (nodehdr.count > args->geo->node_ents) {
> +			xfs_buf_mark_corrupt(blk->bp);
> +			xfs_da_mark_sick(args);
> +			return -EFSCORRUPTED;
> +		}
> +
>  		max = nodehdr.count;
>  		blk->hashval = be32_to_cpu(btree[max - 1].hashval);
>  
> -- 
> 2.53.0
> 
>