[PATCH xfs] xfs: validate remote attr value length in the leaf verifier

[PATCH xfs] xfs: validate remote attr value length in the leaf verifier

[Miao Zhao]

xfs_attr3_leaf_verify_entry() checks a remote attr entry's namelen and
valueblk but never its valuelen. A crafted image can set valuelen to a
value with the high bit set; xfs_attr3_leaf_getvalue() then stores it
in the signed int args->rmtvaluelen, where it goes negative. The
negative length bypasses the -ERANGE size check in xfs_attr_copy_value()
because the comparison is signed, and xfs_attr_rmtval_get() then
reinterprets it as a huge unsigned value, driving the memcpy loop in
xfs_attr_rmtval_copyout() past the end of the getxattr(2) result buffer
with image-controlled data (up to ~64KB on v5, where the remote block
header verifier caps rm_offset + rm_bytes at XFS_XATTR_SIZE_MAX;
unbounded on v4, where remote blocks carry no header at all).

Any unprivileged user with read access to the file can trigger this
with getxattr(2) once a crafted image is mounted, e.g. by a privileged
auto-mount daemon:

 BUG: KASAN: slab-out-of-bounds in xfs_attr_rmtval_copyout
 Write of size 4040 at addr 0000000061bf4ee0 by task xattr_tool/37
 CPU: 0 UID: 65534 PID: 37 Comm: xattr_tool Not tainted 7.2.0-rc5 #1
 Call Trace:
  memcpy
  xfs_attr_rmtval_copyout
  xfs_attr_rmtval_get
  xfs_attr_copy_value
  xfs_attr3_leaf_getvalue
  xfs_attr_node_get
  xfs_attr_get
  xfs_xattr_get
  vfs_getxattr
  sys_getxattr

The same unchecked valuelen is consumed on the INCOMPLETE sweep path
(xfs_attr_inactive.c) to size stale remote blocks, so validate it for
all remote entries: reject valuelen larger than XFS_XATTR_SIZE_MAX,
and reject a zero valuelen on complete entries (a value is only stored
remotely when it is too large for the local format, so zero is never
valid there; INCOMPLETE entries legitimately carry a zeroed valuelen
until the operation completes).

Fixes: c84760659dcf ("xfs: check attribute leaf block structure")
Cc: stable@vger.kernel.org
Signed-off-by: Miao Zhao <muel@nova.gal>
---
 fs/xfs/libxfs/xfs_attr_leaf.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 86c5c09a5db4..b8ae25be639a 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -336,8 +336,11 @@ xfs_attr3_leaf_verify_entry(
 		name_end = (char *)rentry + namesize;
 		if (rentry->namelen == 0)
 			return __this_address;
+		if (be32_to_cpu(rentry->valuelen) > XFS_XATTR_SIZE_MAX)
+			return __this_address;
 		if (!(ent->flags & XFS_ATTR_INCOMPLETE) &&
-		    rentry->valueblk == 0)
+		    (rentry->valueblk == 0 ||
+		     be32_to_cpu(rentry->valuelen) == 0))
 			return __this_address;
 	}
 
-- 
2.34.1

Re: [PATCH xfs] xfs: validate remote attr value length in the leaf verifier

[Darrick J. Wong]

On Tue, Jul 28, 2026 at 06:24:51AM +0000, Miao Zhao wrote:
> xfs_attr3_leaf_verify_entry() checks a remote attr entry's namelen and
> valueblk but never its valuelen. A crafted image can set valuelen to a
> value with the high bit set; xfs_attr3_leaf_getvalue() then stores it
> in the signed int args->rmtvaluelen, where it goes negative. The
> negative length bypasses the -ERANGE size check in xfs_attr_copy_value()
> because the comparison is signed, and xfs_attr_rmtval_get() then
> reinterprets it as a huge unsigned value, driving the memcpy loop in
> xfs_attr_rmtval_copyout() past the end of the getxattr(2) result buffer
> with image-controlled data (up to ~64KB on v5, where the remote block
> header verifier caps rm_offset + rm_bytes at XFS_XATTR_SIZE_MAX;
> unbounded on v4, where remote blocks carry no header at all).
> 
> Any unprivileged user with read access to the file can trigger this
> with getxattr(2) once a crafted image is mounted, e.g. by a privileged
> auto-mount daemon:
> 
>  BUG: KASAN: slab-out-of-bounds in xfs_attr_rmtval_copyout
>  Write of size 4040 at addr 0000000061bf4ee0 by task xattr_tool/37
>  CPU: 0 UID: 65534 PID: 37 Comm: xattr_tool Not tainted 7.2.0-rc5 #1
>  Call Trace:
>   memcpy
>   xfs_attr_rmtval_copyout
>   xfs_attr_rmtval_get
>   xfs_attr_copy_value
>   xfs_attr3_leaf_getvalue
>   xfs_attr_node_get
>   xfs_attr_get
>   xfs_xattr_get
>   vfs_getxattr
>   sys_getxattr
> 
> The same unchecked valuelen is consumed on the INCOMPLETE sweep path
> (xfs_attr_inactive.c) to size stale remote blocks, so validate it for
> all remote entries: reject valuelen larger than XFS_XATTR_SIZE_MAX,
> and reject a zero valuelen on complete entries (a value is only stored
> remotely when it is too large for the local format, so zero is never
> valid there; INCOMPLETE entries legitimately carry a zeroed valuelen
> until the operation completes).
> 
> Fixes: c84760659dcf ("xfs: check attribute leaf block structure")
> Cc: stable@vger.kernel.org
> Signed-off-by: Miao Zhao <muel@nova.gal>

Isn't this a duplicate of
https://lore.kernel.org/linux-xfs/20260707190038.3811440-3-qwe.aldo@gmail.com/#t
?

--D

> ---
>  fs/xfs/libxfs/xfs_attr_leaf.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 86c5c09a5db4..b8ae25be639a 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -336,8 +336,11 @@ xfs_attr3_leaf_verify_entry(
>  		name_end = (char *)rentry + namesize;
>  		if (rentry->namelen == 0)
>  			return __this_address;
> +		if (be32_to_cpu(rentry->valuelen) > XFS_XATTR_SIZE_MAX)
> +			return __this_address;
>  		if (!(ent->flags & XFS_ATTR_INCOMPLETE) &&
> -		    rentry->valueblk == 0)
> +		    (rentry->valueblk == 0 ||
> +		     be32_to_cpu(rentry->valuelen) == 0))
>  			return __this_address;
>  	}
>  
> -- 
> 2.34.1
> 
> 

Re: [PATCH xfs] xfs: validate remote attr value length in the leaf verifier

[MuElnova]

     > Isn't this a duplicate of
     > https://lore.kernel.org/linux-xfs/20260707190038.3811440-3-qwe.aldo@gmail.com/#t
     > ?

     Yes, it is — same root cause and effectively the same verifier check.
     My dedup search was limited to torvalds/linux.git (lore and
     git.kernel.org are unreachable from my test environment), so I missed
     Aldo's series. Sorry for the noise.

     His series looks correct to me, and the 1/2 change to
     xfs_attr_copy_value() closes the signedness bypass at the
     consumption site as well, which my patch did not touch.

     Thanks,
     Miao