Re: [PATCH] fs/xattr.c: fix simple_xattr_list to always include security.* xattrs

From: Paul Moore
Date: Sat Apr 26 2025 - 12:57:12 EST


On Fri, Apr 25, 2025 at 11:14 AM Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> wrote:
> On Fri, Apr 25, 2025 at 5:20 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> > On Thu, Apr 24, 2025 at 11:28:20AM -0400, Stephen Smalley wrote:

...

> > > + if (err < 0)
> > > + return err;
> > > +
> > > + if (buffer) {
> > > + if (remaining_size < err)
> > > + return -ERANGE;
> > > + buffer += err;
> > > + }
> > > + remaining_size -= err;
> >
> > Really unpleasant code duplication in here. We have xattr_list_one() for
> > that. security_inode_listxattr() should probably receive a pointer to
> > &remaining_size?
>
> Not sure how to avoid the duplication, but willing to take it inside
> of security_inode_listsecurity() and change its hook interface if
> desired.

We talked about moving to xattr_list_one() in the other RFC thread
earlier this week and as previously mentioned I think it's the right
thing to do. However, considering the issue with the new coreutils
release, I think it's best to keep this patch limited to the fixes
necessary to restore the desired behavior with the recent coreutils;
this should make life easier for distro and stable backports. We can
address the LSM hook cleanup/rework in a second patch{set} afterwards.

--
paul-moore.com