From: Darrick J. Wong <djwong@xxxxxxxxxx> For NVREPLACEXXX operations, make it possible to log the old and new attr values, since this variant does VLOOKUP operations and we actually need both old and new values to be represented. Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> --- fs/xfs/libxfs/xfs_attr.c | 4 +- fs/xfs/libxfs/xfs_da_btree.h | 2 + fs/xfs/libxfs/xfs_log_format.h | 14 +++++- fs/xfs/xfs_attr_item.c | 88 +++++++++++++++++++++++++++++++++++----- fs/xfs/xfs_attr_item.h | 3 + 5 files changed, 93 insertions(+), 18 deletions(-) diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 35d94608c621..8022e1e1887f 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -432,13 +432,15 @@ xfs_attr_complete_op( /* * NVREPLACE operations require the caller to set the old and new names - * explicitly. + * and values explicitly. */ ASSERT(args->new_namelen > 0); args->name = args->new_name; args->namelen = args->new_namelen; args->hashval = xfs_da_hashname(args->name, args->namelen); + args->value = args->new_value; + args->valuelen = args->new_valuelen; return replace_state; } diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h index 6132787ad1f4..3f3313ace903 100644 --- a/fs/xfs/libxfs/xfs_da_btree.h +++ b/fs/xfs/libxfs/xfs_da_btree.h @@ -60,7 +60,9 @@ typedef struct xfs_da_args { int new_namelen; /* new attr name len */ uint8_t filetype; /* filetype of inode for directories */ void *value; /* set of bytes (maybe contain NULLs) */ + void *new_value; /* new xattr value (may contain NULLs) */ int valuelen; /* length of value */ + int new_valuelen; /* length of new value */ unsigned int attr_filter; /* XFS_ATTR_{ROOT,SECURE,INCOMPLETE} */ unsigned int attr_flags; /* XATTR_{CREATE,REPLACE} */ xfs_dahash_t hashval; /* hash value of name */ diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h index a1581dc6f131..711654fca49a 100644 --- a/fs/xfs/libxfs/xfs_log_format.h +++ b/fs/xfs/libxfs/xfs_log_format.h @@ -115,11 +115,11 @@ struct xfs_unmount_log_format { #define XLOG_REG_TYPE_BUD_FORMAT 26 #define XLOG_REG_TYPE_ATTRI_FORMAT 27 #define XLOG_REG_TYPE_ATTRD_FORMAT 28 -#define XLOG_REG_TYPE_ATTR_NAME 29 +#define XLOG_REG_TYPE_ATTR_NAME 29 #define XLOG_REG_TYPE_ATTR_VALUE 30 #define XLOG_REG_TYPE_ATTR_NEWNAME 31 -#define XLOG_REG_TYPE_MAX 31 - +#define XLOG_REG_TYPE_ATTR_NEWVALUE 32 +#define XLOG_REG_TYPE_MAX 32 /* * Flags to log operation header @@ -980,7 +980,13 @@ struct xfs_icreate_log { struct xfs_attri_log_format { uint16_t alfi_type; /* attri log item type */ uint16_t alfi_size; /* size of this item */ - uint32_t __pad; /* pad to 64 bit aligned */ + + /* + * For NVREPLACEXXX, this is the length of the new xattr value. + * alfi_value_len contains the length of the old xattr value. + */ + uint32_t alfi_newvalue_len; + uint64_t alfi_id; /* attri identifier */ uint64_t alfi_ino; /* the inode for this attr operation */ uint32_t alfi_op_flags; /* marks the op as a set or remove */ diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c index 9eebc9db2280..5497862e4072 100644 --- a/fs/xfs/xfs_attr_item.c +++ b/fs/xfs/xfs_attr_item.c @@ -78,7 +78,9 @@ xfs_attri_log_nameval_alloc( const void *newname, unsigned int newname_len, const void *value, - unsigned int value_len) + unsigned int value_len, + const void *newvalue, + unsigned int newvalue_len) { struct xfs_attri_log_nameval *nv; @@ -87,7 +89,8 @@ xfs_attri_log_nameval_alloc( * this. But kvmalloc() utterly sucks, so we use our own version. */ nv = xlog_kvmalloc(sizeof(struct xfs_attri_log_nameval) + - name_len + newname_len + value_len); + name_len + newname_len + value_len + + newvalue_len); nv->name.i_addr = nv + 1; nv->name.i_len = name_len; @@ -114,6 +117,17 @@ xfs_attri_log_nameval_alloc( } nv->value.i_type = XLOG_REG_TYPE_ATTR_VALUE; + if (newvalue_len) { + nv->newvalue.i_addr = nv->name.i_addr + newname_len + + name_len + value_len; + nv->newvalue.i_len = newvalue_len; + memcpy(nv->newvalue.i_addr, newvalue, newvalue_len); + } else { + nv->newvalue.i_addr = NULL; + nv->newvalue.i_len = 0; + } + nv->newvalue.i_type = XLOG_REG_TYPE_ATTR_NEWVALUE; + refcount_set(&nv->refcount, 1); return nv; } @@ -168,6 +182,11 @@ xfs_attri_item_size( *nvecs += 1; *nbytes += xlog_calc_iovec_len(nv->value.i_len); } + + if (nv->newvalue.i_len) { + *nvecs += 1; + *nbytes += xlog_calc_iovec_len(nv->newvalue.i_len); + } } /* @@ -203,6 +222,9 @@ xfs_attri_item_format( if (nv->value.i_len > 0) attrip->attri_format.alfi_size++; + if (nv->newvalue.i_len > 0) + attrip->attri_format.alfi_size++; + xlog_copy_iovec(lv, &vecp, XLOG_REG_TYPE_ATTRI_FORMAT, &attrip->attri_format, sizeof(struct xfs_attri_log_format)); @@ -213,6 +235,9 @@ xfs_attri_item_format( if (nv->value.i_len > 0) xlog_copy_from_iovec(lv, &vecp, &nv->value); + + if (nv->newvalue.i_len > 0) + xlog_copy_from_iovec(lv, &vecp, &nv->newvalue); } /* @@ -406,6 +431,7 @@ xfs_attr_log_item( if (xfs_attr_log_item_op(attrp) == XFS_ATTRI_OP_FLAGS_NVREPLACEXXX) { attrp->alfi_oldname_len = attr->xattri_nameval->name.i_len; attrp->alfi_newname_len = attr->xattri_nameval->newname.i_len; + attrp->alfi_newvalue_len = attr->xattri_nameval->newvalue.i_len; } else if (xfs_attr_log_item_op(attrp) == XFS_ATTRI_OP_FLAGS_NVREPLACE) { attrp->alfi_oldname_len = attr->xattri_nameval->name.i_len; attrp->alfi_newname_len = attr->xattri_nameval->newname.i_len; @@ -455,7 +481,8 @@ xfs_attr_create_intent( */ attr->xattri_nameval = xfs_attri_log_nameval_alloc(args->name, args->namelen, args->new_name, - args->new_namelen, args->value, args->valuelen); + args->new_namelen, args->value, args->valuelen, + args->new_value, args->new_valuelen); } attrip = xfs_attri_init(mp, attr->xattri_nameval); @@ -556,6 +583,8 @@ xfs_attri_validate( if (attrp->alfi_name_len == 0 || attrp->alfi_name_len > XATTR_NAME_MAX) return false; + if (attrp->alfi_newvalue_len != 0) + return false; break; case XFS_ATTRI_OP_FLAGS_SET: case XFS_ATTRI_OP_FLAGS_REPLACE: @@ -566,6 +595,8 @@ xfs_attri_validate( return false; if (attrp->alfi_value_len > XATTR_SIZE_MAX) return false; + if (attrp->alfi_newvalue_len != 0) + return false; break; case XFS_ATTRI_OP_FLAGS_NVREPLACEXXX: case XFS_ATTRI_OP_FLAGS_NVREPLACE: @@ -577,6 +608,8 @@ xfs_attri_validate( return false; if (attrp->alfi_value_len > XATTR_SIZE_MAX) return false; + if (attrp->alfi_newvalue_len > XATTR_SIZE_MAX) + return false; break; default: return false; @@ -647,6 +680,8 @@ xfs_attri_item_recover( args->hashval = xfs_da_hashname(args->name, args->namelen); args->value = nv->value.i_addr; args->valuelen = nv->value.i_len; + args->new_value = nv->newvalue.i_addr; + args->new_valuelen = nv->newvalue.i_len; args->attr_filter = attrp->alfi_attr_filter & XFS_ATTRI_FILTER_MASK; args->op_flags = XFS_DA_OP_RECOVERY | XFS_DA_OP_OKNOENT | XFS_DA_OP_LOGGED; @@ -753,6 +788,7 @@ xfs_attri_item_relog( if (xfs_attr_log_item_op(old_attrp) == XFS_ATTRI_OP_FLAGS_NVREPLACEXXX) { new_attrp->alfi_newname_len = old_attrp->alfi_newname_len; new_attrp->alfi_oldname_len = old_attrp->alfi_oldname_len; + new_attrp->alfi_newvalue_len = old_attrp->alfi_newvalue_len; } else if (xfs_attr_log_item_op(old_attrp) == XFS_ATTRI_OP_FLAGS_NVREPLACE) { new_attrp->alfi_newname_len = old_attrp->alfi_newname_len; new_attrp->alfi_oldname_len = old_attrp->alfi_oldname_len; @@ -779,11 +815,12 @@ xlog_recover_attri_commit_pass2( struct xfs_attri_log_format *attri_formatp; struct xfs_attri_log_nameval *nv; const void *attr_value = NULL; + const void *attr_newvalue = NULL; const void *attr_name; size_t len; const void *attr_newname = NULL; unsigned int name_len = 0, newname_len = 0; - unsigned int value_len; + unsigned int value_len = 0, newvalue_len = 0; int op, i = 0; /* Validate xfs_attri_log_format before the large memory allocation */ @@ -801,7 +838,6 @@ xlog_recover_attri_commit_pass2( return -EFSCORRUPTED; } - value_len = attri_formatp->alfi_value_len; op = xfs_attr_log_item_op(attri_formatp); switch (op) { case XFS_ATTRI_OP_FLAGS_NVSET: @@ -813,6 +849,7 @@ xlog_recover_attri_commit_pass2( return -EFSCORRUPTED; } name_len = attri_formatp->alfi_name_len; + value_len = attri_formatp->alfi_value_len; break; case XFS_ATTRI_OP_FLAGS_SET: case XFS_ATTRI_OP_FLAGS_REPLACE: @@ -823,6 +860,7 @@ xlog_recover_attri_commit_pass2( return -EFSCORRUPTED; } name_len = attri_formatp->alfi_name_len; + value_len = attri_formatp->alfi_value_len; break; case XFS_ATTRI_OP_FLAGS_REMOVE: /* Log item, attr name */ @@ -834,14 +872,19 @@ xlog_recover_attri_commit_pass2( name_len = attri_formatp->alfi_name_len; break; case XFS_ATTRI_OP_FLAGS_NVREPLACEXXX: - /* Log item, attr name, new attr name, optional attr value */ - if (item->ri_total < 3 || item->ri_total > 4) { + /* + * Log item, attr name, new attr name, optional attr value, + * optional new attr value + */ + if (item->ri_total < 3 || item->ri_total > 5) { XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, attri_formatp, len); return -EFSCORRUPTED; } name_len = attri_formatp->alfi_oldname_len; newname_len = attri_formatp->alfi_newname_len; + value_len = attri_formatp->alfi_value_len; + newvalue_len = attri_formatp->alfi_newvalue_len; break; case XFS_ATTRI_OP_FLAGS_NVREPLACE: /* Log item, attr name, new attr name, attr value */ @@ -852,6 +895,7 @@ xlog_recover_attri_commit_pass2( } name_len = attri_formatp->alfi_oldname_len; newname_len = attri_formatp->alfi_newname_len; + value_len = attri_formatp->alfi_value_len; break; default: XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, @@ -896,9 +940,8 @@ xlog_recover_attri_commit_pass2( i++; } - /* Validate the attr value, if present */ - if (value_len != 0) { + if (value_len > 0) { if (item->ri_buf[i].i_len != xlog_calc_iovec_len(value_len)) { XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, attri_formatp, len); @@ -909,6 +952,18 @@ xlog_recover_attri_commit_pass2( i++; } + /* Validate the new attr value, if present */ + if (newvalue_len > 0) { + if (item->ri_buf[i].i_len != xlog_calc_iovec_len(newvalue_len)) { + XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, + attri_formatp, len); + return -EFSCORRUPTED; + } + + attr_newvalue = item->ri_buf[i].i_addr; + i++; + } + /* * Make sure we got the correct number of buffers for the operation * that we just loaded. @@ -947,10 +1002,18 @@ xlog_recover_attri_commit_pass2( } break; case XFS_ATTRI_OP_FLAGS_NVREPLACE: + /* Old name-value replace operations do not have new values. */ + if (attr_newvalue != NULL || newvalue_len != 0) { + XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, + attri_formatp, len); + return -EFSCORRUPTED; + } + fallthrough; case XFS_ATTRI_OP_FLAGS_NVREPLACEXXX: /* - * Name-value replace operations require the caller to specify - * the old and new name explicitly. Values are optional. + * New name-value replace operations require the caller to + * specify the old and new names and values explicitly. + * Values are optional. */ if (attr_name == NULL || name_len == 0) { XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp, @@ -971,7 +1034,8 @@ xlog_recover_attri_commit_pass2( * reference. */ nv = xfs_attri_log_nameval_alloc(attr_name, name_len, attr_newname, - newname_len, attr_value, value_len); + newname_len, attr_value, value_len, attr_newvalue, + newvalue_len); attrip = xfs_attri_init(mp, nv); memcpy(&attrip->attri_format, attri_formatp, len); diff --git a/fs/xfs/xfs_attr_item.h b/fs/xfs/xfs_attr_item.h index e374712ba06b..098f7f3aae78 100644 --- a/fs/xfs/xfs_attr_item.h +++ b/fs/xfs/xfs_attr_item.h @@ -13,8 +13,9 @@ struct kmem_zone; struct xfs_attri_log_nameval { struct xfs_log_iovec name; - struct xfs_log_iovec newname; + struct xfs_log_iovec newname; /* NVREPLACEXXX only */ struct xfs_log_iovec value; + struct xfs_log_iovec newvalue; /* NVREPLACEXXX only */ refcount_t refcount; /* name and value follow the end of this struct */