On Thu, Apr 24, 2025 at 11:28:20AM -0400, Stephen Smalley wrote: > The vfs has long had a fallback to obtain the security.* xattrs from the > LSM when the filesystem does not implement its own listxattr, but > shmem/tmpfs and kernfs later gained their own xattr handlers to support > other xattrs. Unfortunately, as a side effect, tmpfs and kernfs-based This change is from 2011. So no living soul has ever cared at all for at least 14 years. Surprising that this is an issue now. > filesystems like sysfs no longer return the synthetic security.* xattr > names via listxattr unless they are explicitly set by userspace or > initially set upon inode creation after policy load. coreutils has > recently switched from unconditionally invoking getxattr for security.* > for ls -Z via libselinux to only doing so if listxattr returns the xattr > name, breaking ls -Z of such inodes. So no xattrs have been set on a given inode and we lie to userspace by listing them anyway. Well ok then. > Before: > $ getfattr -m.* /run/initramfs > <no output> > $ getfattr -m.* /sys/kernel/fscaps > <no output> > $ setfattr -n user.foo /run/initramfs > $ getfattr -m.* /run/initramfs > user.foo > > After: > $ getfattr -m.* /run/initramfs > security.selinux > $ getfattr -m.* /sys/kernel/fscaps > security.selinux > $ setfattr -n user.foo /run/initramfs > $ getfattr -m.* /run/initramfs > security.selinux > user.foo > > Link: https://lore.kernel.org/selinux/CAFqZXNtF8wDyQajPCdGn=iOawX4y77ph0EcfcqcUUj+T87FKyA@xxxxxxxxxxxxxx/ > Link: https://lore.kernel.org/selinux/20250423175728.3185-2-stephen.smalley.work@xxxxxxxxx/ > Signed-off-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx> > --- > fs/xattr.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/fs/xattr.c b/fs/xattr.c > index 02bee149ad96..2fc314b27120 100644 > --- a/fs/xattr.c > +++ b/fs/xattr.c > @@ -1428,6 +1428,15 @@ static bool xattr_is_trusted(const char *name) > return !strncmp(name, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN); > } > > +static bool xattr_is_maclabel(const char *name) > +{ > + const char *suffix = name + XATTR_SECURITY_PREFIX_LEN; > + > + return !strncmp(name, XATTR_SECURITY_PREFIX, > + XATTR_SECURITY_PREFIX_LEN) && > + security_ismaclabel(suffix); > +} > + > /** > * simple_xattr_list - list all xattr objects > * @inode: inode from which to get the xattrs > @@ -1460,6 +1469,17 @@ ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs, > if (err) > return err; > > + err = security_inode_listsecurity(inode, buffer, remaining_size); Is that supposed to work with multiple LSMs? Afaict, bpf is always active and has a hook for this. So the LSMs trample over each other filling the buffer? > + 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? > + > read_lock(&xattrs->lock); > for (rbp = rb_first(&xattrs->rb_root); rbp; rbp = rb_next(rbp)) { > xattr = rb_entry(rbp, struct simple_xattr, rb_node); > @@ -1468,6 +1488,10 @@ ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs, > if (!trusted && xattr_is_trusted(xattr->name)) > continue; > > + /* skip MAC labels; these are provided by LSM above */ > + if (xattr_is_maclabel(xattr->name)) > + continue; > + > err = xattr_list_one(&buffer, &remaining_size, xattr->name); > if (err) > break; > -- > 2.49.0 >
