[PATCH] xfs: reject remote xattr entries with an out-of-range value length

[PATCH] xfs: reject remote xattr entries with an out-of-range value length

[Aldo Ariel Panzardo]

xfs_attr3_leaf_verify_entry() validates a remote attribute entry's name
but never bounds its on-disk value length (xfs_attr_leaf_name_remote.
valuelen, a __be32). A crafted leaf with valuelen = 0x80000000 passes
the verifier and the CRC.

That length is later assigned into the signed int args->rmtvaluelen
(xfs_attr3_leaf_getvalue), becoming negative, which slips past the signed
-ERANGE check in xfs_attr_copy_value(); a getxattr() with a small buffer
then memcpy()s a full remote block into the small kvalue buffer
(xfs_attr_rmtval_copyout) -- a heap out-of-bounds write with
attacker-controlled content, from an unprivileged getxattr(2) on a
mounted crafted image.

Reject remote entries whose value length exceeds XFS_XATTR_SIZE_MAX in
the leaf verifier, so the malicious block is rejected at read time.

Fixes: c84760659dcf ("xfs: check attribute leaf block structure")
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 fs/xfs/libxfs/xfs_attr_leaf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 86c5c09a5db4..f1769289f908 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -339,6 +339,8 @@ xfs_attr3_leaf_verify_entry(
 		if (!(ent->flags & XFS_ATTR_INCOMPLETE) &&
 		    rentry->valueblk == 0)
 			return __this_address;
+		if (be32_to_cpu(rentry->valuelen) > XFS_XATTR_SIZE_MAX)
+			return __this_address;
 	}
 
 	if (name_end > buf_end)
-- 
2.43.0

Re: [PATCH] xfs: reject remote xattr entries with an out-of-range value length

[Darrick J. Wong]

On Tue, Jul 07, 2026 at 11:01:18AM -0300, Aldo Ariel Panzardo wrote:
> xfs_attr3_leaf_verify_entry() validates a remote attribute entry's name
> but never bounds its on-disk value length (xfs_attr_leaf_name_remote.
> valuelen, a __be32). A crafted leaf with valuelen = 0x80000000 passes
> the verifier and the CRC.
> 
> That length is later assigned into the signed int args->rmtvaluelen
> (xfs_attr3_leaf_getvalue), becoming negative, which slips past the signed
> -ERANGE check in xfs_attr_copy_value(); a getxattr() with a small buffer

Shouldn't you fix the signed value check in xfs_attr_copy_value, then?

> then memcpy()s a full remote block into the small kvalue buffer
> (xfs_attr_rmtval_copyout) -- a heap out-of-bounds write with
> attacker-controlled content, from an unprivileged getxattr(2) on a
> mounted crafted image.
> 
> Reject remote entries whose value length exceeds XFS_XATTR_SIZE_MAX in
> the leaf verifier, so the malicious block is rejected at read time.
> 
> Fixes: c84760659dcf ("xfs: check attribute leaf block structure")
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

This ought to cc stable.

> ---
>  fs/xfs/libxfs/xfs_attr_leaf.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 86c5c09a5db4..f1769289f908 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -339,6 +339,8 @@ xfs_attr3_leaf_verify_entry(
>  		if (!(ent->flags & XFS_ATTR_INCOMPLETE) &&
>  		    rentry->valueblk == 0)
>  			return __this_address;
> +		if (be32_to_cpu(rentry->valuelen) > XFS_XATTR_SIZE_MAX)
> +			return __this_address;

This looks correct though.

--D

>  	}
>  
>  	if (name_end > buf_end)
> -- 
> 2.43.0
> 
> 

[PATCH v2 0/2] xfs: fix out-of-range remote xattr value length

[Aldo Ariel Panzardo]

v1 tightened only the read-time leaf verifier.  Darrick pointed out that
the underlying problem is the signed value-length check in the consumer,
xfs_attr_copy_value(): a remote value length is an on-disk __be32 that is
stored into the signed args->rmtvaluelen, so a crafted length such as
0x80000000 becomes negative and slips past the "buffer too small"
-ERANGE guard, and is then used as a copy length.

v2 fixes both ends:

  1/2 rejects a negative or oversized value length in
      xfs_attr_copy_value() before it is used, so a bogus length cannot
      become a copy length even if it reaches the value copier.
  2/2 is the v1 verifier fix, rejecting the malformed remote entry at
      read time.

Both are bug fixes and are now cc'd to stable.

v2:
  - 1/2 is new: fix the signed value-length check in xfs_attr_copy_value
    (the consumer / root cause), per Darrick's review.
  - 2/2: cc stable; otherwise unchanged from the v1 verifier patch.

v1: https://lore.kernel.org/linux-xfs/20260707140118.3217585-1-qwe.aldo@gmail.com/

Aldo Ariel Panzardo (2):
  xfs: reject out-of-range attribute value lengths in
    xfs_attr_copy_value
  xfs: reject remote xattr entries with an out-of-range value length

 fs/xfs/libxfs/xfs_attr_leaf.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

-- 
2.53.0

[PATCH v2 1/2] xfs: reject out-of-range attribute value lengths in xfs_attr_copy_value

[Aldo Ariel Panzardo]

xfs_attr_copy_value() takes the value length as a signed int and, for a
remote xattr, is handed args->rmtvaluelen.  That field is filled from the
on-disk __be32 xfs_attr_leaf_name_remote.valuelen in
xfs_attr3_leaf_getvalue(), so a crafted length such as 0x80000000 is
stored into the signed rmtvaluelen as a negative number.

The "buffer too small" guard in xfs_attr_copy_value() is a signed
comparison:

	if (args->valuelen < valuelen)
		return -ERANGE;

A negative valuelen therefore compares as smaller than the caller's
buffer size, skips the -ERANGE path, and is then used as a copy length,
leading to an out-of-bounds copy of a full remote block into a small
getxattr(2) buffer on a mounted crafted image.

Reject a value length that is negative or larger than the maximum xattr
size before it is used, so a bogus on-disk length can no longer slip
through the value copier.

Fixes: 9df243a1a9e6 ("xfs: consolidate attribute value copying")
Cc: <stable@vger.kernel.org>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
v2: new patch (see the 0/2 cover letter).  Fixes the signed value-length
    check in the consumer, xfs_attr_copy_value(), which is the root
    cause Darrick pointed at in his review of the v1 verifier patch.

 fs/xfs/libxfs/xfs_attr_leaf.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 86c5c09a5db4..d0f7753659c9 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -628,6 +628,17 @@ xfs_attr_copy_value(
 	unsigned char		*value,
 	int			valuelen)
 {
+	/*
+	 * A value length that is negative or larger than the maximum xattr
+	 * size is on-disk corruption.  The remote value length is an on-disk
+	 * __be32 stored into the signed args->rmtvaluelen, so a crafted value
+	 * such as 0x80000000 becomes negative and would slip past the
+	 * "args->valuelen < valuelen" check below and be used as a copy
+	 * length.  Reject it before that can happen.
+	 */
+	if (valuelen < 0 || valuelen > XFS_XATTR_SIZE_MAX)
+		return -EFSCORRUPTED;
+
 	/*
 	 * Parent pointer lookups require the caller to specify the name and
 	 * value, so don't copy anything.
-- 
2.53.0

[PATCH v2 2/2] xfs: reject remote xattr entries with an out-of-range value length

[Aldo Ariel Panzardo]

xfs_attr3_leaf_verify_entry() validates a remote attribute entry's name
but never bounds its on-disk value length (xfs_attr_leaf_name_remote.
valuelen, a __be32). A crafted leaf with valuelen = 0x80000000 passes
the verifier and the CRC.

That length is later assigned into the signed int args->rmtvaluelen
(xfs_attr3_leaf_getvalue), becoming negative, which slips past the signed
-ERANGE check in xfs_attr_copy_value(); a getxattr() with a small buffer
then memcpy()s a full remote block into the small kvalue buffer
(xfs_attr_rmtval_copyout) -- a heap out-of-bounds write with
attacker-controlled content, from an unprivileged getxattr(2) on a
mounted crafted image.

Reject remote entries whose value length exceeds XFS_XATTR_SIZE_MAX in
the leaf verifier, so the malicious block is rejected at read time.

Fixes: c84760659dcf ("xfs: check attribute leaf block structure")
Cc: <stable@vger.kernel.org>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
v2: cc stable (per Darrick).  Now 2/2 of a series: 1/2 fixes the signed
    value-length check in xfs_attr_copy_value(); this read-time verifier
    fix is otherwise unchanged from v1.

 fs/xfs/libxfs/xfs_attr_leaf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index d0f7753659c9..948dc8b26fe6 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -339,6 +339,8 @@ xfs_attr3_leaf_verify_entry(
 		if (!(ent->flags & XFS_ATTR_INCOMPLETE) &&
 		    rentry->valueblk == 0)
 			return __this_address;
+		if (be32_to_cpu(rentry->valuelen) > XFS_XATTR_SIZE_MAX)
+			return __this_address;
 	}
 
 	if (name_end > buf_end)
-- 
2.53.0