Some additional comments on this new one. I think maybe this should
have been an rfc since xfs_attr_list doesnt have any existing
filtration logic. So with out assuming what behavior people would
prefer it to have, I've applied the minimal amount of change that
solves the problem for now. Suggestions welcome though, I assumed
probably people will want to discuss other solutions during the review.
Thanks!
Allison
On Wed, 2022-09-21 at 22:44 -0700, allison.henderson@xxxxxxxxxx wrote:
> From: Allison Henderson <allison.henderson@xxxxxxxxxx>
>
> Parent pointers returned to the get_fattr tool cause errors since
> the tool cannot parse parent pointers. Fix this by filtering parent
> parent pointers from xfs_attr_list.
>
> Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_da_format.h | 3 +++
> fs/xfs/xfs_attr_list.c | 47 +++++++++++++++++++++++++++------
> --
> 2 files changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_da_format.h
> b/fs/xfs/libxfs/xfs_da_format.h
> index b02b67f1999e..e9c323fab6f3 100644
> --- a/fs/xfs/libxfs/xfs_da_format.h
> +++ b/fs/xfs/libxfs/xfs_da_format.h
> @@ -697,6 +697,9 @@ struct xfs_attr3_leafblock {
> #define XFS_ATTR_INCOMPLETE (1u << XFS_ATTR_INCOMPLETE_BIT)
> #define XFS_ATTR_NSP_ONDISK_MASK \
> (XFS_ATTR_ROOT | XFS_ATTR_SECURE |
> XFS_ATTR_PARENT)
> +#define XFS_ATTR_ALL \
> + (XFS_ATTR_LOCAL_BIT | XFS_ATTR_ROOT | XFS_ATTR_SECURE | \
> + XFS_ATTR_PARENT | XFS_ATTR_INCOMPLETE_BIT)
>
> /*
> * Alignment for namelist and valuelist entries (since they are
> mixed
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index a51f7f13a352..13de597c4996 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -39,6 +39,23 @@ xfs_attr_shortform_compare(const void *a, const
> void *b)
> }
> }
>
> +/*
> + * Returns true or false if the parent attribute should be listed
> + */
> +static bool
> +xfs_attr_filter_parent(
> + struct xfs_attr_list_context *context,
> + int flags)
> +{
> + if (!(flags & XFS_ATTR_PARENT))
> + return true;
> +
> + if (context->attr_filter & XFS_ATTR_PARENT)
> + return true;
> +
> + return false;
> +}
> +
> #define XFS_ISRESET_CURSOR(cursor) \
> (!((cursor)->initted) && !((cursor)->hashval) && \
> !((cursor)->blkno) && !((cursor)->offset))
> @@ -90,11 +107,12 @@ xfs_attr_shortform_list(
> sfe-
> >namelen,
> sfe-
> >flags)))
> return -EFSCORRUPTED;
> - context->put_listent(context,
> - sfe->flags,
> - sfe->nameval,
> - (int)sfe->namelen,
> - (int)sfe->valuelen);
> + if (xfs_attr_filter_parent(context, sfe-
> >flags))
> + context->put_listent(context,
> + sfe->flags,
> + sfe->nameval,
> + (int)sfe-
> >namelen,
> + (int)sfe-
> >valuelen);
> /*
> * Either search callback finished early or
> * didn't fit it all in the buffer after all.
> @@ -185,11 +203,12 @@ xfs_attr_shortform_list(
> error = -EFSCORRUPTED;
> goto out;
> }
> - context->put_listent(context,
> - sbp->flags,
> - sbp->name,
> - sbp->namelen,
> - sbp->valuelen);
> + if (xfs_attr_filter_parent(context, sbp->flags))
> + context->put_listent(context,
> + sbp->flags,
> + sbp->name,
> + sbp->namelen,
> + sbp->valuelen);
> if (context->seen_enough)
> break;
> cursor->offset++;
> @@ -474,8 +493,10 @@ xfs_attr3_leaf_list_int(
> !xfs_attr_namecheck(mp, name,
> namelen,
> entry-
> >flags)))
> return -EFSCORRUPTED;
> - context->put_listent(context, entry->flags,
> + if (xfs_attr_filter_parent(context, entry->flags))
> + context->put_listent(context, entry->flags,
> name, namelen,
> valuelen);
> +
> if (context->seen_enough)
> break;
> cursor->offset++;
> @@ -539,6 +560,10 @@ xfs_attr_list(
> if (xfs_is_shutdown(dp->i_mount))
> return -EIO;
>
> + if (context->attr_filter == 0)
> + context->attr_filter =
> + XFS_ATTR_ALL & ~XFS_ATTR_PARENT;
> +
> lock_mode = xfs_ilock_attr_map_shared(dp);
> error = xfs_attr_list_ilocked(context);
> xfs_iunlock(dp, lock_mode);
