Skip to content

Commit 1c7f09d

Browse files
author
Darrick J. Wong
committed
xfs: validate recovered name buffers when recovering xattr items
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@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de>
1 parent 2a2c05d commit 1c7f09d

File tree

1 file changed

+47
-11
lines changed

1 file changed

+47
-11
lines changed

fs/xfs/xfs_attr_item.c

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -741,22 +741,20 @@ xlog_recover_attri_commit_pass2(
741741
const void *attr_value = NULL;
742742
const void *attr_name;
743743
size_t len;
744-
unsigned int op;
745-
746-
attri_formatp = item->ri_buf[0].i_addr;
747-
attr_name = item->ri_buf[1].i_addr;
744+
unsigned int op, i = 0;
748745

749746
/* Validate xfs_attri_log_format before the large memory allocation */
750747
len = sizeof(struct xfs_attri_log_format);
751-
if (item->ri_buf[0].i_len != len) {
748+
if (item->ri_buf[i].i_len != len) {
752749
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
753750
item->ri_buf[0].i_addr, item->ri_buf[0].i_len);
754751
return -EFSCORRUPTED;
755752
}
756753

754+
attri_formatp = item->ri_buf[i].i_addr;
757755
if (!xfs_attri_validate(mp, attri_formatp)) {
758756
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
759-
item->ri_buf[0].i_addr, item->ri_buf[0].i_len);
757+
attri_formatp, len);
760758
return -EFSCORRUPTED;
761759
}
762760

@@ -785,31 +783,69 @@ xlog_recover_attri_commit_pass2(
785783
attri_formatp, len);
786784
return -EFSCORRUPTED;
787785
}
786+
i++;
788787

789788
/* Validate the attr name */
790-
if (item->ri_buf[1].i_len !=
789+
if (item->ri_buf[i].i_len !=
791790
xlog_calc_iovec_len(attri_formatp->alfi_name_len)) {
792791
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
793-
item->ri_buf[0].i_addr, item->ri_buf[0].i_len);
792+
attri_formatp, len);
794793
return -EFSCORRUPTED;
795794
}
796795

796+
attr_name = item->ri_buf[i].i_addr;
797797
if (!xfs_attr_namecheck(attr_name, attri_formatp->alfi_name_len)) {
798798
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
799-
item->ri_buf[1].i_addr, item->ri_buf[1].i_len);
799+
attri_formatp, len);
800800
return -EFSCORRUPTED;
801801
}
802+
i++;
802803

803804
/* Validate the attr value, if present */
804805
if (attri_formatp->alfi_value_len != 0) {
805-
if (item->ri_buf[2].i_len != xlog_calc_iovec_len(attri_formatp->alfi_value_len)) {
806+
if (item->ri_buf[i].i_len != xlog_calc_iovec_len(attri_formatp->alfi_value_len)) {
806807
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
807808
item->ri_buf[0].i_addr,
808809
item->ri_buf[0].i_len);
809810
return -EFSCORRUPTED;
810811
}
811812

812-
attr_value = item->ri_buf[2].i_addr;
813+
attr_value = item->ri_buf[i].i_addr;
814+
i++;
815+
}
816+
817+
/*
818+
* Make sure we got the correct number of buffers for the operation
819+
* that we just loaded.
820+
*/
821+
if (i != item->ri_total) {
822+
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
823+
attri_formatp, len);
824+
return -EFSCORRUPTED;
825+
}
826+
827+
switch (op) {
828+
case XFS_ATTRI_OP_FLAGS_REMOVE:
829+
/* Regular remove operations operate only on names. */
830+
if (attr_value != NULL || attri_formatp->alfi_value_len != 0) {
831+
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
832+
attri_formatp, len);
833+
return -EFSCORRUPTED;
834+
}
835+
fallthrough;
836+
case XFS_ATTRI_OP_FLAGS_SET:
837+
case XFS_ATTRI_OP_FLAGS_REPLACE:
838+
/*
839+
* Regular xattr set/remove/replace operations require a name
840+
* and do not take a newname. Values are optional for set and
841+
* replace.
842+
*/
843+
if (attr_name == NULL || attri_formatp->alfi_name_len == 0) {
844+
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
845+
attri_formatp, len);
846+
return -EFSCORRUPTED;
847+
}
848+
break;
813849
}
814850

815851
/*

0 commit comments

Comments
 (0)