Re: [PATCH v2] security,fs,nfs,net: update security_inode_listsecurity() interface

[Konstantin Andreev <andreev@swemel.ru>]

Stephen Smalley, 06/06/2025 10:28 -0400:
> On Fri, Jun 6, 2025 at 9:38 AM Konstantin Andreev <andreev@swemel.ru> wrote:
>> Stephen Smalley, 28/04/2025:
>>> Update the security_inode_listsecurity() interface to allow
>>> use of the xattr_list_one() helper and update the hook
>>> implementations.
>>>
>>> Link: https://lore.kernel.org/selinux/20250424152822.2719-1-stephen.smalley.work@gmail.com/
>>
>> Sorry for being late to the party.
>>
>> Your approach assumes that every fs-specific xattr lister
>> called like
>>
>> | vfs_listxattr() {
>> |    if (inode->i_op->listxattr)
>> |        error = inode->i_op->listxattr(dentry, list, size)
>> |   ...
>>
>> must call LSM to integrate LSM's xattr(s) into fs-specific list.
>> You did this for tmpfs:
>>
>> | simple_xattr_list() {
>> |   security_inode_listsecurity()
>> |   // iterate real xatts list
>>
>>
>> Well, but what about other filesystems in the linux kernel?
>> Should all of them also modify their xattr listers?
>>
>> To me, taking care of security xattrs is improper responsibility
>> for filesystem code.
>>
>> May it be better to merge LSM xattrs
>> and fs-backed xattrs at the vfs level (vfs_listxattr)?
> 
> This patch and the preceding one on which it depends were specifically
> to address a regression in the handling of listxattr() for tmpfs/shmem
> and similar filesystems.
> Originally they had no xattr handler at the filesystem level and
> vfs_listxattr() already has a fallback to ensure inclusion of the
> security.* xattr for that case.

Understood

> For filesystems like ext4 that have always (relative to first
> introduction of security.* xattrs) provided handlers, they already
> return the fs-backed xattr value and we don't need to ask the LSM for
> it.

They only return those security.* xattrs that were physically stored
in the fs permanent storage.

If LSM's xattrs are not stored they are not listed :(

> That said, you may be correct that it would be better to introduce
> some additional handling in vfs_listxattr() but I would recommend
> doing that as a follow-up.

Understood

--
Konstantin Andreev