From: "Darrick J. Wong" <djwong@xxxxxxxxxx> commit 1c7f09d210aba2f2bb206e2e8c97c9f11a3fd880 upstream. 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 <djwong@xxxxxxxxxx> Reviewed-by: Christoph Hellwig <hch@xxxxxx> Signed-off-by: Catherine Hoang <catherine.hoang@xxxxxxxxxx> Acked-by: Darrick J. Wong <djwong@xxxxxxxxxx> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- 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 @@ -719,22 +719,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; } @@ -763,31 +761,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 catherine.hoang@xxxxxxxxxx are queue-6.6/xfs-enforce-one-namespace-per-attribute.patch queue-6.6/xfs-use-dontcache-for-grabbing-inodes-during-scrub.patch queue-6.6/xfs-revert-commit-44af6c7e59b12.patch queue-6.6/xfs-check-shortform-attr-entry-flags-specifically.patch queue-6.6/xfs-make-the-seq-argument-to-xfs_bmapi_convert_delalloc-optional.patch queue-6.6/xfs-check-opcode-and-iovec-count-match-in-xlog_recover_attri_commit_pass2.patch queue-6.6/xfs-allow-symlinks-with-short-remote-targets.patch queue-6.6/xfs-match-lock-mode-in-xfs_buffered_write_iomap_begin.patch queue-6.6/xfs-require-xfs_sb_feat_incompat_log_xattrs-for-attr-log-intent-item-recovery.patch queue-6.6/xfs-allow-unlinked-symlinks-and-dirs-with-zero-size.patch queue-6.6/xfs-restrict-when-we-try-to-align-cow-fork-delalloc-to-cowextsz-hints.patch queue-6.6/xfs-fix-xfs_bmap_add_extent_delay_real-for-partial-conversions.patch queue-6.6/xfs-fix-missing-check-for-invalid-attr-flags.patch queue-6.6/xfs-validate-recovered-name-buffers-when-recovering-xattr-items.patch queue-6.6/xfs-make-xfs_bmapi_convert_delalloc-to-allocate-the-target-offset.patch queue-6.6/xfs-convert-delayed-extents-to-unwritten-when-zeroing-post-eof-blocks.patch queue-6.6/xfs-fix-freeing-speculative-preallocations-for-preallocated-files.patch queue-6.6/xfs-fix-unlink-vs-cluster-buffer-instantiation-race.patch queue-6.6/xfs-make-sure-sb_fdblocks-is-non-negative.patch queue-6.6/xfs-remove-a-racy-if_bytes-check-in-xfs_reflink_end_cow_extent.patch queue-6.6/xfs-fix-error-returns-from-xfs_bmapi_write.patch