[PATCH] xfs: reject attr leaf blocks with inconsistent usedbytes

[PATCH] xfs: reject attr leaf blocks with inconsistent usedbytes

[Weiming Shi]

xfs_attr3_leaf_verify() checks each attr leaf entry on its own, but never
checks that the entries' nameval regions are disjoint. A crafted leaf can
point several entries at overlapping offsets: every entry passes the
per-entry check, yet the summed entry sizes far exceed the nameval region.

ichdr.usedbytes is kept as the exact sum of the entries'
xfs_attr_leaf_entsize() (see xfs_attr3_leaf_add()), so for such a leaf the
real sum no longer matches usedbytes. When the leaf is later repacked,
xfs_attr3_leaf_compact() resets firstused to blksize and calls
xfs_attr3_leaf_moveents(), which subtracts each entry size from firstused;
the oversized sum underflows the 32-bit firstused and the following memmove
writes out of bounds. The same repack runs from xfs_attr3_leaf_rebalance()
and xfs_attr3_leaf_unbalance(). The only guard is an ASSERT, which is
compiled out on production kernels.

A single setxattr() on a file with such a leaf, after mounting a crafted
image, triggers the write:

  BUG: KASAN: use-after-free in xfs_attr3_leaf_moveents (fs/xfs/libxfs/xfs_attr_leaf.c:2788)
  Write of size 400 at addr ffff88802b187f98 by task exploit
   xfs_attr3_leaf_moveents (fs/xfs/libxfs/xfs_attr_leaf.c:2788)
   xfs_attr3_leaf_compact (fs/xfs/libxfs/xfs_attr_leaf.c:1790)
   xfs_attr3_leaf_add (fs/xfs/libxfs/xfs_attr_leaf.c:1563)
   xfs_attr_set_iter (fs/xfs/libxfs/xfs_attr.c:556)
   xfs_attr_set (fs/xfs/libxfs/xfs_attr.c:1244)
   xfs_xattr_set (fs/xfs/xfs_xattr.c:186)
   __vfs_setxattr (fs/xattr.c:218)
   vfs_setxattr (fs/xattr.c:339)
   __x64_sys_fsetxattr (fs/xattr.c:774)

Sum the entry sizes while verifying and reject the leaf unless the sum
equals usedbytes and usedbytes fits in [firstused, blksize).  The online
scrubber already validates this in xchk_xattr_block(); this brings the
read/write verifier in line with it so the bad leaf is rejected before any
reshape can run.

Fixes: c84760659dcf ("xfs: check attribute leaf block structure")
Reported-by: Xiang Mei <xmei5@asu.edu>
Assisted-by: Claude:claude-opus-4-8
Cc: stable@vger.kernel.org
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 fs/xfs/libxfs/xfs_attr_leaf.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 86c5c09a5db4..9814dcfbd7ac 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -300,7 +300,8 @@ xfs_attr3_leaf_verify_entry(
 	struct xfs_attr3_icleaf_hdr		*leafhdr,
 	struct xfs_attr_leaf_entry		*ent,
 	int					idx,
-	__u32					*last_hashval)
+	__u32					*last_hashval,
+	unsigned int				*usedbytes)
 {
 	struct xfs_attr_leaf_name_local		*lentry;
 	struct xfs_attr_leaf_name_remote	*rentry;
@@ -344,6 +345,7 @@ xfs_attr3_leaf_verify_entry(
 	if (name_end > buf_end)
 		return __this_address;
 
+	*usedbytes += namesize;
 	return NULL;
 }
 
@@ -376,6 +378,7 @@ xfs_attr3_leaf_verify(
 	char				*buf_end;
 	uint32_t			end;	/* must be 32bit - see below */
 	__u32				last_hashval = 0;
+	unsigned int			usedbytes = 0;
 	int				i;
 	xfs_failaddr_t			fa;
 
@@ -410,11 +413,21 @@ xfs_attr3_leaf_verify(
 	buf_end = (char *)bp->b_addr + mp->m_attr_geo->blksize;
 	for (i = 0, ent = entries; i < ichdr.count; ent++, i++) {
 		fa = xfs_attr3_leaf_verify_entry(mp, buf_end, leaf, &ichdr,
-				ent, i, &last_hashval);
+				ent, i, &last_hashval, &usedbytes);
 		if (fa)
 			return fa;
 	}
 
+	/*
+	 * usedbytes must equal the summed entry sizes and fit in the
+	 * nameval region; otherwise a later repack underflows firstused
+	 * in xfs_attr3_leaf_moveents().
+	 */
+	if (usedbytes != ichdr.usedbytes)
+		return __this_address;
+	if (ichdr.usedbytes > mp->m_attr_geo->blksize - ichdr.firstused)
+		return __this_address;
+
 	/*
 	 * Quickly check the freemap information.  Attribute data has to be
 	 * aligned to 4-byte boundaries, and likewise for the free space.
-- 
2.43.0

Re: [PATCH] xfs: reject attr leaf blocks with inconsistent usedbytes

[Darrick J. Wong]

On Fri, Jul 03, 2026 at 08:15:44AM -0700, Weiming Shi wrote:
> xfs_attr3_leaf_verify() checks each attr leaf entry on its own, but never
> checks that the entries' nameval regions are disjoint. A crafted leaf can
> point several entries at overlapping offsets: every entry passes the
> per-entry check, yet the summed entry sizes far exceed the nameval region.
> 
> ichdr.usedbytes is kept as the exact sum of the entries'
> xfs_attr_leaf_entsize() (see xfs_attr3_leaf_add()), so for such a leaf the
> real sum no longer matches usedbytes. When the leaf is later repacked,
> xfs_attr3_leaf_compact() resets firstused to blksize and calls
> xfs_attr3_leaf_moveents(), which subtracts each entry size from firstused;
> the oversized sum underflows the 32-bit firstused and the following memmove
> writes out of bounds. The same repack runs from xfs_attr3_leaf_rebalance()
> and xfs_attr3_leaf_unbalance(). The only guard is an ASSERT, which is
> compiled out on production kernels.
> 
> A single setxattr() on a file with such a leaf, after mounting a crafted
> image, triggers the write:
> 
>   BUG: KASAN: use-after-free in xfs_attr3_leaf_moveents (fs/xfs/libxfs/xfs_attr_leaf.c:2788)
>   Write of size 400 at addr ffff88802b187f98 by task exploit
>    xfs_attr3_leaf_moveents (fs/xfs/libxfs/xfs_attr_leaf.c:2788)
>    xfs_attr3_leaf_compact (fs/xfs/libxfs/xfs_attr_leaf.c:1790)
>    xfs_attr3_leaf_add (fs/xfs/libxfs/xfs_attr_leaf.c:1563)
>    xfs_attr_set_iter (fs/xfs/libxfs/xfs_attr.c:556)
>    xfs_attr_set (fs/xfs/libxfs/xfs_attr.c:1244)
>    xfs_xattr_set (fs/xfs/xfs_xattr.c:186)
>    __vfs_setxattr (fs/xattr.c:218)
>    vfs_setxattr (fs/xattr.c:339)
>    __x64_sys_fsetxattr (fs/xattr.c:774)
> 
> Sum the entry sizes while verifying and reject the leaf unless the sum
> equals usedbytes and usedbytes fits in [firstused, blksize).  The online
> scrubber already validates this in xchk_xattr_block(); this brings the
> read/write verifier in line with it so the bad leaf is rejected before any
> reshape can run.
> 
> Fixes: c84760659dcf ("xfs: check attribute leaf block structure")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Assisted-by: Claude:claude-opus-4-8
> Cc: stable@vger.kernel.org
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
>  fs/xfs/libxfs/xfs_attr_leaf.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 86c5c09a5db4..9814dcfbd7ac 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -300,7 +300,8 @@ xfs_attr3_leaf_verify_entry(
>  	struct xfs_attr3_icleaf_hdr		*leafhdr,
>  	struct xfs_attr_leaf_entry		*ent,
>  	int					idx,
> -	__u32					*last_hashval)
> +	__u32					*last_hashval,
> +	unsigned int				*usedbytes)
>  {
>  	struct xfs_attr_leaf_name_local		*lentry;
>  	struct xfs_attr_leaf_name_remote	*rentry;
> @@ -344,6 +345,7 @@ xfs_attr3_leaf_verify_entry(
>  	if (name_end > buf_end)
>  		return __this_address;
>  
> +	*usedbytes += namesize;
>  	return NULL;
>  }
>  
> @@ -376,6 +378,7 @@ xfs_attr3_leaf_verify(
>  	char				*buf_end;
>  	uint32_t			end;	/* must be 32bit - see below */
>  	__u32				last_hashval = 0;
> +	unsigned int			usedbytes = 0;
>  	int				i;
>  	xfs_failaddr_t			fa;
>  
> @@ -410,11 +413,21 @@ xfs_attr3_leaf_verify(
>  	buf_end = (char *)bp->b_addr + mp->m_attr_geo->blksize;
>  	for (i = 0, ent = entries; i < ichdr.count; ent++, i++) {
>  		fa = xfs_attr3_leaf_verify_entry(mp, buf_end, leaf, &ichdr,
> -				ent, i, &last_hashval);
> +				ent, i, &last_hashval, &usedbytes);
>  		if (fa)
>  			return fa;
>  	}
>  
> +	/*
> +	 * usedbytes must equal the summed entry sizes and fit in the
> +	 * nameval region; otherwise a later repack underflows firstused
> +	 * in xfs_attr3_leaf_moveents().
> +	 */
> +	if (usedbytes != ichdr.usedbytes)
> +		return __this_address;
> +	if (ichdr.usedbytes > mp->m_attr_geo->blksize - ichdr.firstused)

Interesting ... where did this new logic come from?  xchk_xattr_block
doesn't perform this test.

--D

> +		return __this_address;
> +
>  	/*
>  	 * Quickly check the freemap information.  Attribute data has to be
>  	 * aligned to 4-byte boundaries, and likewise for the free space.
> -- 
> 2.43.0
> 
> 

Re: [PATCH] xfs: reject attr leaf blocks with inconsistent usedbytes

[Weiming Shi]

Darrick J. Wong <djwong@kernel.org> 于2026年7月8日周三 00:34写道：
>
> On Fri, Jul 03, 2026 at 08:15:44AM -0700, Weiming Shi wrote:
> > xfs_attr3_leaf_verify() checks each attr leaf entry on its own, but never
> > checks that the entries' nameval regions are disjoint. A crafted leaf can
> > point several entries at overlapping offsets: every entry passes the
> > per-entry check, yet the summed entry sizes far exceed the nameval region.
> >
> > ichdr.usedbytes is kept as the exact sum of the entries'
> > xfs_attr_leaf_entsize() (see xfs_attr3_leaf_add()), so for such a leaf the
> > real sum no longer matches usedbytes. When the leaf is later repacked,
> > xfs_attr3_leaf_compact() resets firstused to blksize and calls
> > xfs_attr3_leaf_moveents(), which subtracts each entry size from firstused;
> > the oversized sum underflows the 32-bit firstused and the following memmove
> > writes out of bounds. The same repack runs from xfs_attr3_leaf_rebalance()
> > and xfs_attr3_leaf_unbalance(). The only guard is an ASSERT, which is
> > compiled out on production kernels.
> >
> > A single setxattr() on a file with such a leaf, after mounting a crafted
> > image, triggers the write:
> >
> >   BUG: KASAN: use-after-free in xfs_attr3_leaf_moveents (fs/xfs/libxfs/xfs_attr_leaf.c:2788)
> >   Write of size 400 at addr ffff88802b187f98 by task exploit
> >    xfs_attr3_leaf_moveents (fs/xfs/libxfs/xfs_attr_leaf.c:2788)
> >    xfs_attr3_leaf_compact (fs/xfs/libxfs/xfs_attr_leaf.c:1790)
> >    xfs_attr3_leaf_add (fs/xfs/libxfs/xfs_attr_leaf.c:1563)
> >    xfs_attr_set_iter (fs/xfs/libxfs/xfs_attr.c:556)
> >    xfs_attr_set (fs/xfs/libxfs/xfs_attr.c:1244)
> >    xfs_xattr_set (fs/xfs/xfs_xattr.c:186)
> >    __vfs_setxattr (fs/xattr.c:218)
> >    vfs_setxattr (fs/xattr.c:339)
> >    __x64_sys_fsetxattr (fs/xattr.c:774)
> >
> > Sum the entry sizes while verifying and reject the leaf unless the sum
> > equals usedbytes and usedbytes fits in [firstused, blksize).  The online
> > scrubber already validates this in xchk_xattr_block(); this brings the
> > read/write verifier in line with it so the bad leaf is rejected before any
> > reshape can run.
> >
> > Fixes: c84760659dcf ("xfs: check attribute leaf block structure")
> > Reported-by: Xiang Mei <xmei5@asu.edu>
> > Assisted-by: Claude:claude-opus-4-8
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > ---
> >  fs/xfs/libxfs/xfs_attr_leaf.c | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> > index 86c5c09a5db4..9814dcfbd7ac 100644
> > --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> > @@ -300,7 +300,8 @@ xfs_attr3_leaf_verify_entry(
> >       struct xfs_attr3_icleaf_hdr             *leafhdr,
> >       struct xfs_attr_leaf_entry              *ent,
> >       int                                     idx,
> > -     __u32                                   *last_hashval)
> > +     __u32                                   *last_hashval,
> > +     unsigned int                            *usedbytes)
> >  {
> >       struct xfs_attr_leaf_name_local         *lentry;
> >       struct xfs_attr_leaf_name_remote        *rentry;
> > @@ -344,6 +345,7 @@ xfs_attr3_leaf_verify_entry(
> >       if (name_end > buf_end)
> >               return __this_address;
> >
> > +     *usedbytes += namesize;
> >       return NULL;
> >  }
> >
> > @@ -376,6 +378,7 @@ xfs_attr3_leaf_verify(
> >       char                            *buf_end;
> >       uint32_t                        end;    /* must be 32bit - see below */
> >       __u32                           last_hashval = 0;
> > +     unsigned int                    usedbytes = 0;
> >       int                             i;
> >       xfs_failaddr_t                  fa;
> >
> > @@ -410,11 +413,21 @@ xfs_attr3_leaf_verify(
> >       buf_end = (char *)bp->b_addr + mp->m_attr_geo->blksize;
> >       for (i = 0, ent = entries; i < ichdr.count; ent++, i++) {
> >               fa = xfs_attr3_leaf_verify_entry(mp, buf_end, leaf, &ichdr,
> > -                             ent, i, &last_hashval);
> > +                             ent, i, &last_hashval, &usedbytes);
> >               if (fa)
> >                       return fa;
> >       }
> >
> > +     /*
> > +      * usedbytes must equal the summed entry sizes and fit in the
> > +      * nameval region; otherwise a later repack underflows firstused
> > +      * in xfs_attr3_leaf_moveents().
> > +      */
> > +     if (usedbytes != ichdr.usedbytes)
> > +             return __this_address;
> > +     if (ichdr.usedbytes > mp->m_attr_geo->blksize - ichdr.firstused)
>
> Interesting ... where did this new logic come from?  xchk_xattr_block
> doesn't perform this test.
>
> --D
>
> > +             return __this_address;
> > +
> >       /*
> >        * Quickly check the freemap information.  Attribute data has to be
> >        * aligned to 4-byte boundaries, and likewise for the free space.
> > --
> > 2.43.0
> >
> >

Yeah, you're right, sorry. scrub only checks "usedbytes == summed sizes"
  (my first check). It has nothing for the second one,

        if (ichdr.usedbytes > mp->m_attr_geo->blksize - ichdr.firstused)

  The closest scrub test is "usedbytes > blksize", which bounds usedbytes
  against the whole block, not the nameval region, so it wouldn't catch this.
  Saying the scrubber already validates it was just wrong.

  My reproducer's leaf (firstused=3616, real summed entsize=6000, usedbytes
  field=8) is actually caught by the first check, since 6000 != 8. But the
  first check alone isn't enough: an image that instead sets the usedbytes
  field to the oversized real sum passes the equality, and then only the
  second check (6000 > 4096-3616) rejects it before moveents underflows
  firstused. So both are needed.

  I'll drop the scrubber reference in v2. Code's unchanged. Thanks.