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

[PATCH] 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 it does not know which
tree the block belongs to. When the directory block size exceeds the fs
block size (e.g. mkfs.xfs -n size=64k -b size=4k), the attribute node
buffer is a single fs block holding only m_attr_geo->node_ents entries,
but a crafted attr node may claim a count up to m_dir_geo->node_ents.
xfs_da3_node_lookup_int() then reads btree[] entries past the buffer
during its binary search -- an out-of-bounds read via getxattr/listxattr
on a mounted crafted image.

The node buffer size identifies its geometry, so bound the count against
that geometry's node_ents rather than the maximum of the two.

Fixes: 7ab610f9e0f1 ("xfs: move node entry counts to xfs_da_geometry")
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 fs/xfs/libxfs/xfs_da_btree.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 9debb95d86fa..897c31147a46 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -240,12 +240,18 @@ xfs_da3_node_verify(
 		return __this_address;
 
 	/*
-	 * we don't know if the node is for and attribute or directory tree,
-	 * so only fail if the count is outside both bounds
+	 * The block was read using either the attribute or the directory
+	 * geometry; its buffer size tells us which one, so bound the entry
+	 * count against that geometry's node_ents.  Only failing when the
+	 * count exceeds max(dir, attr) let a crafted attr node claim a
+	 * dir-sized count and overrun the smaller attr buffer.
 	 */
-	if (ichdr.count > mp->m_dir_geo->node_ents &&
-	    ichdr.count > mp->m_attr_geo->node_ents)
+	if (BBTOB(bp->b_length) == mp->m_attr_geo->blksize) {
+		if (ichdr.count > mp->m_attr_geo->node_ents)
+			return __this_address;
+	} else if (ichdr.count > mp->m_dir_geo->node_ents) {
 		return __this_address;
+	}
 
 	/* XXX: hash order check? */
 
-- 
2.43.0

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

[Darrick J. Wong]

On Tue, Jul 07, 2026 at 10:59:30AM -0300, Aldo Ariel Panzardo wrote:
> xfs_da3_node_verify() bounds the node entry count against the larger of
> the directory and attribute geometries because it does not know which
> tree the block belongs to. When the directory block size exceeds the fs
> block size (e.g. mkfs.xfs -n size=64k -b size=4k), the attribute node
> buffer is a single fs block holding only m_attr_geo->node_ents entries,
> but a crafted attr node may claim a count up to m_dir_geo->node_ents.
> xfs_da3_node_lookup_int() then reads btree[] entries past the buffer
> during its binary search -- an out-of-bounds read via getxattr/listxattr
> on a mounted crafted image.
> 
> The node buffer size identifies its geometry, so bound the count against
> that geometry's node_ents rather than the maximum of the two.
> 
> Fixes: 7ab610f9e0f1 ("xfs: move node entry counts to xfs_da_geometry")
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_da_btree.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 9debb95d86fa..897c31147a46 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -240,12 +240,18 @@ xfs_da3_node_verify(
>  		return __this_address;
>  
>  	/*
> -	 * we don't know if the node is for and attribute or directory tree,
> -	 * so only fail if the count is outside both bounds
> +	 * The block was read using either the attribute or the directory
> +	 * geometry; its buffer size tells us which one, so bound the entry
> +	 * count against that geometry's node_ents.  Only failing when the
> +	 * count exceeds max(dir, attr) let a crafted attr node claim a
> +	 * dir-sized count and overrun the smaller attr buffer.
>  	 */
> -	if (ichdr.count > mp->m_dir_geo->node_ents &&
> -	    ichdr.count > mp->m_attr_geo->node_ents)
> +	if (BBTOB(bp->b_length) == mp->m_attr_geo->blksize) {

No.  That's not how we determine if the caller is trying to read a
directory or an xattr block.

--D

> +		if (ichdr.count > mp->m_attr_geo->node_ents)
> +			return __this_address;
> +	} else if (ichdr.count > mp->m_dir_geo->node_ents) {
>  		return __this_address;
> +	}
>  
>  	/* XXX: hash order check? */
>  
> -- 
> 2.43.0
> 
> 

[PATCH v2] 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] xfs: bound da-node entry count against the correct geometry

[Carlos Maiolino]

On Tue, Jul 07, 2026 at 04:02:45PM -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.

For all the patches, Vx shouldn't be in-reply-to to the initial
versions. While this is 'acceptable' it really makes it hard to me to
track down what I have yet to process because the threading. You don't
have to re-send the by now, but once you get RwBs (or need to send a V3)
please resubmit them with the tags (or the newer versions) without
replying to the original.

> 
> 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
> 
>