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

[PATCH v2 RESEND 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 RESEND 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

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

[Darrick J. Wong]

On Wed, Sep 23, 2026 at 08:58:22AM -0300, Aldo Ariel Panzardo wrote:
> 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>

I wonder if we /really/ need this check now that the leaf verifier is
getting fixed, but I do see the value in making sure that crazy sizes
don't get passed to kvmalloc later on, so:

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

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

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

[Darrick J. Wong]

On Wed, Sep 23, 2026 at 08:58:23AM -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
> 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>

Still looks good,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

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