Re: [PATCH 23/32] xfs: Filter XFS_ATTR_PARENT for getfattr

Darrick J. Wong
2024-04-10T21:58:28Z

On Tue, Apr 09, 2024 at 10:51:18PM -0700, Christoph Hellwig wrote:
> On Tue, Apr 09, 2024 at 05:59:30PM -0700, Darrick J. Wong wrote:
> > From: Allison Henderson <allison.henderson@oracle.com>
> >
> > 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_xattr_put_listent.
>
> With the new format returning the attrs should not cause parsing errors.
> OTOH we now have duplicate names, which means a get operation based on
> the name can't actually work in that case.
>
> I'd also argue that parent pointers are internal enough that they
> should not be exposed through the normal xattr interfaces.

Yeah, I probably should change the commit message to:

"xfs: don't return XFS_ATTR_PARENT attributes via listxattr

"Parent pointers are internal filesystem metadata. They're not intended
to be directly visible to userspace, so filter them out of
xfs_xattr_put_listent so that they don't appear in listxattr."

> > +/*
> > + * This file defines functions to work with externally visible extended
> > + * attributes, such as those in user, system, or security namespaces. They
> > + * should not be used for internally used attributes. Consider xfs_attr.c.
> > + */
>
> As long as xfs_attr_change and xfs_attr_grab_log_assist are xfs_xattr.c
> that is not actually true. However I think they should be moved to
> xfs_attr.c (and in case of xfs_attr_change merged into xfs_attr_set)
> to make this comment true.

I don't want to hoist all the larp enabling jun^Wmachinery to libxfs and
then have to stub that out in userspace. I'd rather get rid of larp
mode entirely, after which point xfs_attr_change becomes a trivial
helper that can be collapsed.

> However I'd make it part of the top of file comment above the include
> statements. And please add it in a separate commit as it has nothing
> to do with the other changes here.

Or just get rid of the comment entirely? It came from the verity
series.

--D