Patch "xfs: validate recovered name buffers when recovering xattr items" has been added to the 6.1-stable tree

This is a note to let you know that I've just added the patch titled

    xfs: validate recovered name buffers when recovering xattr items

to the 6.1-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     xfs-validate-recovered-name-buffers-when-recovering-xattr-items.patch
and it can be found in the queue-6.1 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <[email protected]> know about it.


From [email protected] Wed Apr 30 23:33:05 2025
From: Leah Rumancik <[email protected]>
Date: Wed, 30 Apr 2025 14:26:53 -0700
Subject: xfs: validate recovered name buffers when recovering xattr items
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], Christoph Hellwig <[email protected]>, Leah Rumancik <[email protected]>
Message-ID: <[email protected]>

From: "Darrick J. Wong" <[email protected]>

[ Upstream commit 1c7f09d210aba2f2bb206e2e8c97c9f11a3fd880 ]

Strengthen the xattri log item recovery code by checking that we
actually have the required name and newname buffers for whatever
operation we're replaying.

Signed-off-by: Darrick J. Wong <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Leah Rumancik <[email protected]>
Acked-by: "Darrick J. Wong" <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
 fs/xfs/xfs_attr_item.c |   58 +++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 11 deletions(-)

--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -717,22 +717,20 @@ xlog_recover_attri_commit_pass2(
 	const void			*attr_value = NULL;
 	const void			*attr_name;
 	size_t				len;
-	unsigned int			op;
-
-	attri_formatp = item->ri_buf[0].i_addr;
-	attr_name = item->ri_buf[1].i_addr;
+	unsigned int			op, i = 0;
 
 	/* Validate xfs_attri_log_format before the large memory allocation */
 	len = sizeof(struct xfs_attri_log_format);
-	if (item->ri_buf[0].i_len != len) {
+	if (item->ri_buf[i].i_len != len) {
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
 				item->ri_buf[0].i_addr, item->ri_buf[0].i_len);
 		return -EFSCORRUPTED;
 	}
 
+	attri_formatp = item->ri_buf[i].i_addr;
 	if (!xfs_attri_validate(mp, attri_formatp)) {
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
-				item->ri_buf[0].i_addr, item->ri_buf[0].i_len);
+				attri_formatp, len);
 		return -EFSCORRUPTED;
 	}
 
@@ -761,31 +759,69 @@ xlog_recover_attri_commit_pass2(
 				     attri_formatp, len);
 		return -EFSCORRUPTED;
 	}
+	i++;
 
 	/* Validate the attr name */
-	if (item->ri_buf[1].i_len !=
+	if (item->ri_buf[i].i_len !=
 			xlog_calc_iovec_len(attri_formatp->alfi_name_len)) {
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
-				item->ri_buf[0].i_addr, item->ri_buf[0].i_len);
+				attri_formatp, len);
 		return -EFSCORRUPTED;
 	}
 
+	attr_name = item->ri_buf[i].i_addr;
 	if (!xfs_attr_namecheck(attr_name, attri_formatp->alfi_name_len)) {
 		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
-				item->ri_buf[1].i_addr, item->ri_buf[1].i_len);
+				attri_formatp, len);
 		return -EFSCORRUPTED;
 	}
+	i++;
 
 	/* Validate the attr value, if present */
 	if (attri_formatp->alfi_value_len != 0) {
-		if (item->ri_buf[2].i_len != xlog_calc_iovec_len(attri_formatp->alfi_value_len)) {
+		if (item->ri_buf[i].i_len != xlog_calc_iovec_len(attri_formatp->alfi_value_len)) {
 			XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
 					item->ri_buf[0].i_addr,
 					item->ri_buf[0].i_len);
 			return -EFSCORRUPTED;
 		}
 
-		attr_value = item->ri_buf[2].i_addr;
+		attr_value = item->ri_buf[i].i_addr;
+		i++;
+	}
+
+	/*
+	 * Make sure we got the correct number of buffers for the operation
+	 * that we just loaded.
+	 */
+	if (i != item->ri_total) {
+		XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+				attri_formatp, len);
+		return -EFSCORRUPTED;
+	}
+
+	switch (op) {
+	case XFS_ATTRI_OP_FLAGS_REMOVE:
+		/* Regular remove operations operate only on names. */
+		if (attr_value != NULL || attri_formatp->alfi_value_len != 0) {
+			XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+					     attri_formatp, len);
+			return -EFSCORRUPTED;
+		}
+		fallthrough;
+	case XFS_ATTRI_OP_FLAGS_SET:
+	case XFS_ATTRI_OP_FLAGS_REPLACE:
+		/*
+		 * Regular xattr set/remove/replace operations require a name
+		 * and do not take a newname.  Values are optional for set and
+		 * replace.
+		 */
+		if (attr_name == NULL || attri_formatp->alfi_name_len == 0) {
+			XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+					     attri_formatp, len);
+			return -EFSCORRUPTED;
+		}
+		break;
 	}
 
 	/*


Patches currently in stable-queue which might be from [email protected] are

queue-6.1/xfs-revert-commit-44af6c7e59b12.patch
queue-6.1/xfs-make-the-seq-argument-to-xfs_bmapi_convert_delalloc-optional.patch
queue-6.1/xfs-check-opcode-and-iovec-count-match-in-xlog_recover_attri_commit_pass2.patch
queue-6.1/xfs-allow-symlinks-with-short-remote-targets.patch
queue-6.1/xfs-match-lock-mode-in-xfs_buffered_write_iomap_begin.patch
queue-6.1/xfs-require-xfs_sb_feat_incompat_log_xattrs-for-attr-log-intent-item-recovery.patch
queue-6.1/xfs-allow-unlinked-symlinks-and-dirs-with-zero-size.patch
queue-6.1/xfs-restrict-when-we-try-to-align-cow-fork-delalloc-to-cowextsz-hints.patch
queue-6.1/xfs-fix-xfs_bmap_add_extent_delay_real-for-partial-conversions.patch
queue-6.1/xfs-validate-recovered-name-buffers-when-recovering-xattr-items.patch
queue-6.1/xfs-make-xfs_bmapi_convert_delalloc-to-allocate-the-target-offset.patch
queue-6.1/xfs-convert-delayed-extents-to-unwritten-when-zeroing-post-eof-blocks.patch
queue-6.1/xfs-fix-freeing-speculative-preallocations-for-preallocated-files.patch
queue-6.1/xfs-make-sure-sb_fdblocks-is-non-negative.patch
queue-6.1/xfs-remove-a-racy-if_bytes-check-in-xfs_reflink_end_cow_extent.patch
queue-6.1/xfs-fix-error-returns-from-xfs_bmapi_write.patch