On Sun, 2022-05-15 at 20:32 -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> Make sure we screen the "attr flags" field of recovered xattr intent
> log
> items to reject flag bits that we don't know about. This is really
> the
> attr *filter* flags, so rename the field and create properly
> namespaced
> flags to fill it.
>
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_log_format.h | 9 ++++++++-
> fs/xfs/xfs_attr_item.c | 10 +++++++---
> 2 files changed, 15 insertions(+), 4 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_log_format.h
> b/fs/xfs/libxfs/xfs_log_format.h
> index f7edd1ecf6d9..5017500bfd8b 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -911,6 +911,13 @@ struct xfs_icreate_log {
> #define XFS_ATTR_OP_FLAGS_REPLACE 3 /* Replace the attribute */
> #define XFS_ATTR_OP_FLAGS_TYPE_MASK 0xFF /* Flags type mask */
>
> +#define XFS_ATTRI_FILTER_ROOT (1u <<
> XFS_ATTR_ROOT_BIT)
> +#define XFS_ATTRI_FILTER_SECURE (1u <<
> XFS_ATTR_SECURE_BIT)
> +#define XFS_ATTRI_FILTER_INCOMPLETE (1u << XFS_ATTR_INCOMPLETE_BIT)
> +#define XFS_ATTRI_FILTER_MASK (XFS_ATTRI_FILTER_ROOT
> | \
> + XFS_ATTRI_FILTER_SECURE | \
> + XFS_ATTRI_FILTER_INCOMPLETE)
> +
It sounds like your already working on a v2 that doesnt use the new
flag scheme, but other than that, I think it looks ok. Thanks!
Allison
> /*
> * This is the structure used to lay out an attr log item in the
> * log.
> @@ -924,7 +931,7 @@ struct xfs_attri_log_format {
> uint32_t alfi_op_flags; /* marks the op as a set or remove */
> uint32_t alfi_name_len; /* attr name length */
> uint32_t alfi_value_len; /* attr value length */
> - uint32_t alfi_attr_flags;/* attr flags */
> + uint32_t alfi_attr_filter;/* attr filter flags */
> };
>
> struct xfs_attrd_log_format {
> diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
> index 459b6c93b40b..7cbb640d7856 100644
> --- a/fs/xfs/xfs_attr_item.c
> +++ b/fs/xfs/xfs_attr_item.c
> @@ -353,7 +353,8 @@ xfs_attr_log_item(
> XFS_ATTR_OP_FLAGS_TYPE_
> MASK;
> attrp->alfi_value_len = attr->xattri_da_args->valuelen;
> attrp->alfi_name_len = attr->xattri_da_args->namelen;
> - attrp->alfi_attr_flags = attr->xattri_da_args->attr_filter;
> + attrp->alfi_attr_filter = attr->xattri_da_args->attr_filter &
> + XFS_ATTRI_FILTER_MASK;
>
> memcpy(attrip->attri_name, attr->xattri_da_args->name,
> attr->xattri_da_args->namelen);
> @@ -500,6 +501,9 @@ xfs_attri_validate(
> if (attrp->alfi_op_flags & ~XFS_ATTR_OP_FLAGS_TYPE_MASK)
> return false;
>
> + if (attrp->alfi_attr_filter & ~XFS_ATTRI_FILTER_MASK)
> + return false;
> +
> /* alfi_op_flags should be either a set or remove */
> switch (op) {
> case XFS_ATTR_OP_FLAGS_SET:
> @@ -569,7 +573,7 @@ xfs_attri_item_recover(
> args->name = attrip->attri_name;
> args->namelen = attrp->alfi_name_len;
> args->hashval = xfs_da_hashname(args->name, args->namelen);
> - args->attr_filter = attrp->alfi_attr_flags;
> + args->attr_filter = attrp->alfi_attr_filter &
> XFS_ATTRI_FILTER_MASK;
> args->op_flags = XFS_DA_OP_RECOVERY | XFS_DA_OP_OKNOENT;
>
> switch (attr->xattri_op_flags) {
> @@ -658,7 +662,7 @@ xfs_attri_item_relog(
> new_attrp->alfi_op_flags = old_attrp->alfi_op_flags;
> new_attrp->alfi_value_len = old_attrp->alfi_value_len;
> new_attrp->alfi_name_len = old_attrp->alfi_name_len;
> - new_attrp->alfi_attr_flags = old_attrp->alfi_attr_flags;
> + new_attrp->alfi_attr_filter = old_attrp->alfi_attr_filter;
>
> memcpy(new_attrip->attri_name, old_attrip->attri_name,
> new_attrip->attri_name_len);
>
