Skip to content

Commit 9716cdc

Browse files
Darrick J. Wonggregkh
authored andcommitted
xfs: validate recovered name buffers when recovering xattr items
commit 1c7f09d 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@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com> Acked-by: Darrick J. Wong <djwong@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent db460c2 commit 9716cdc

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
@@ -719,22 +719,20 @@ xlog_recover_attri_commit_pass2(
719719
const void *attr_value = NULL;
720720
const void *attr_name;
721721
size_t len;
722-
unsigned int op;
723-
724-
attri_formatp = item->ri_buf[0].i_addr;
725-
attr_name = item->ri_buf[1].i_addr;
722+
unsigned int op, i = 0;
726723

727724
/* Validate xfs_attri_log_format before the large memory allocation */
728725
len = sizeof(struct xfs_attri_log_format);
729-
if (item->ri_buf[0].i_len != len) {
726+
if (item->ri_buf[i].i_len != len) {
730727
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
731728
item->ri_buf[0].i_addr, item->ri_buf[0].i_len);
732729
return -EFSCORRUPTED;
733730
}
734731

732+
attri_formatp = item->ri_buf[i].i_addr;
735733
if (!xfs_attri_validate(mp, attri_formatp)) {
736734
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
737-
item->ri_buf[0].i_addr, item->ri_buf[0].i_len);
735+
attri_formatp, len);
738736
return -EFSCORRUPTED;
739737
}
740738

@@ -763,31 +761,69 @@ xlog_recover_attri_commit_pass2(
763761
attri_formatp, len);
764762
return -EFSCORRUPTED;
765763
}
764+
i++;
766765

767766
/* Validate the attr name */
768-
if (item->ri_buf[1].i_len !=
767+
if (item->ri_buf[i].i_len !=
769768
xlog_calc_iovec_len(attri_formatp->alfi_name_len)) {
770769
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
771-
item->ri_buf[0].i_addr, item->ri_buf[0].i_len);
770+
attri_formatp, len);
772771
return -EFSCORRUPTED;
773772
}
774773

774+
attr_name = item->ri_buf[i].i_addr;
775775
if (!xfs_attr_namecheck(attr_name, attri_formatp->alfi_name_len)) {
776776
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
777-
item->ri_buf[1].i_addr, item->ri_buf[1].i_len);
777+
attri_formatp, len);
778778
return -EFSCORRUPTED;
779779
}
780+
i++;
780781

781782
/* Validate the attr value, if present */
782783
if (attri_formatp->alfi_value_len != 0) {
783-
if (item->ri_buf[2].i_len != xlog_calc_iovec_len(attri_formatp->alfi_value_len)) {
784+
if (item->ri_buf[i].i_len != xlog_calc_iovec_len(attri_formatp->alfi_value_len)) {
784785
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
785786
item->ri_buf[0].i_addr,
786787
item->ri_buf[0].i_len);
787788
return -EFSCORRUPTED;
788789
}
789790

790-
attr_value = item->ri_buf[2].i_addr;
791+
attr_value = item->ri_buf[i].i_addr;
792+
i++;
793+
}
794+
795+
/*
796+
* Make sure we got the correct number of buffers for the operation
797+
* that we just loaded.
798+
*/
799+
if (i != item->ri_total) {
800+
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
801+
attri_formatp, len);
802+
return -EFSCORRUPTED;
803+
}
804+
805+
switch (op) {
806+
case XFS_ATTRI_OP_FLAGS_REMOVE:
807+
/* Regular remove operations operate only on names. */
808+
if (attr_value != NULL || attri_formatp->alfi_value_len != 0) {
809+
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
810+
attri_formatp, len);
811+
return -EFSCORRUPTED;
812+
}
813+
fallthrough;
814+
case XFS_ATTRI_OP_FLAGS_SET:
815+
case XFS_ATTRI_OP_FLAGS_REPLACE:
816+
/*
817+
* Regular xattr set/remove/replace operations require a name
818+
* and do not take a newname. Values are optional for set and
819+
* replace.
820+
*/
821+
if (attr_name == NULL || attri_formatp->alfi_name_len == 0) {
822+
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
823+
attri_formatp, len);
824+
return -EFSCORRUPTED;
825+
}
826+
break;
791827
}
792828

793829
/*

0 commit comments

Comments
 (0)