[PATCH] xfs: fix a UAF problem in xattr repair

[PATCH] xfs: fix a UAF problem in xattr repair

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

The xchk_setup_xattr_buf function can allocate a new value buffer, which
means that any reference to ab->value before the call could become a
dangling pointer.  Fix this by moving an assignment to after the buffer
setup.

Cc: <stable@vger.kernel.org> # v6.10
Fixes: e47dcf113ae348 ("xfs: repair extended attributes")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/scrub/attr_repair.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/scrub/attr_repair.c b/fs/xfs/scrub/attr_repair.c
index c7eb94069cafcd..09d63aa10314b0 100644
--- a/fs/xfs/scrub/attr_repair.c
+++ b/fs/xfs/scrub/attr_repair.c
@@ -333,7 +333,6 @@ xrep_xattr_salvage_remote_attr(
 		.attr_filter		= ent->flags & XFS_ATTR_NSP_ONDISK_MASK,
 		.namelen		= rentry->namelen,
 		.name			= rentry->name,
-		.value			= ab->value,
 		.valuelen		= be32_to_cpu(rentry->valuelen),
 	};
 	unsigned int			namesize;
@@ -363,6 +362,7 @@ xrep_xattr_salvage_remote_attr(
 		error = -EDEADLOCK;
 	if (error)
 		return error;
+	args.value = ab->value;
 
 	/* Look up the remote value and stash it for reconstruction. */
 	error = xfs_attr3_leaf_getvalue(leaf_bp, &args);

Re: [PATCH] xfs: fix a UAF problem in xattr repair

[Christoph Hellwig]

On Thu, Dec 04, 2025 at 01:43:50PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> The xchk_setup_xattr_buf function can allocate a new value buffer, which
> means that any reference to ab->value before the call could become a
> dangling pointer.  Fix this by moving an assignment to after the buffer
> setup.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Do you have a test case for this?

Re: [PATCH] xfs: fix a UAF problem in xattr repair

[Darrick J. Wong]

On Fri, Dec 05, 2025 at 12:05:45AM -0800, Christoph Hellwig wrote:
> On Thu, Dec 04, 2025 at 01:43:50PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > The xchk_setup_xattr_buf function can allocate a new value buffer, which
> > means that any reference to ab->value before the call could become a
> > dangling pointer.  Fix this by moving an assignment to after the buffer
> > setup.
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Do you have a test case for this?

Not yet, but working on it.  I'm working on a corruption problem I
occasionally see in the xattr leaf freemap code, so I think the trick is
that you have to generate a leaf block with a corruption that will cause
the scrubber to exit early having called xchk_*_set_corrupt, and then a
remote xattr with a value larger than xfs_attr_leaf_entsize_local_max.

Obviously this is easy to reproduce after I added a verifier check for
corrupt freemap data, but for older kernels I'd need to inject some
sort of corruption.  Probably twiddling the crc or something would
suffice, but ATM I'm a little absorbed in sorting out the freemap code
and stamping out the bugs.

--D