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