On Wed, Dec 3, 2025 at 1:08 PM Stephen Smalley
[off-list ref] wrote:

On Wed, Dec 3, 2025 at 10:55 AM Paul Moore [off-list ref] wrote:

quoted

On Wed, Dec 3, 2025 at 10:35 AM Stephen Smalley
[off-list ref] wrote:

quoted

On Wed, Jul 23, 2025 at 10:10 PM Paul Moore [off-list ref] wrote:

quoted

On Thu, Jun 19, 2025 at 5:18 PM Paul Moore [off-list ref] wrote:

quoted

On Tue, May 27, 2025 at 5:03 PM Anna Schumaker
[off-list ref] wrote:

quoted

On 5/20/25 5:31 PM, Paul Moore wrote:

quoted

On Tue, Apr 29, 2025 at 7:34 PM Paul Moore [off-list ref] wrote:

quoted

On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley
[off-list ref] wrote:

quoted

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/ (local)

Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
---
This patch is relative to the one linked above, which in theory is on
vfs.fixes but doesn't appear to have been pushed when I looked.

 fs/nfs/nfs4proc.c             | 10 ++++++----
 fs/xattr.c                    | 19 +++++++------------
 include/linux/lsm_hook_defs.h |  4 ++--
 include/linux/security.h      |  5 +++--
 net/socket.c                  | 17 +++++++----------
 security/security.c           | 16 ++++++++--------
 security/selinux/hooks.c      | 10 +++-------
 security/smack/smack_lsm.c    | 13 ++++---------
 8 files changed, 40 insertions(+), 54 deletions(-)

Thanks Stephen.  Once we get ACKs from the NFS, netdev, and Smack
folks I can pull this into the LSM tree.

Gentle ping for Trond, Anna, Jakub, and Casey ... can I get some ACKs
on this patch?  It's a little late for the upcoming merge window, but
I'd like to merge this via the LSM tree after the merge window closes.

For the NFS change:
    Acked-by: Anna Schumaker [off-list ref]

Hi Anna,

Thanks for reviewing the patch.  Unfortunately when merging the patch
today and fixing up some merge conflicts I bumped into an odd case in
the NFS space and I wanted to check with you on how you would like to
resolve it.

Commit 243fea134633 ("NFSv4.2: fix listxattr to return selinux
security label")[1] adds a direct call to
security_inode_listsecurity() in nfs4_listxattr(), despite the
existing nfs4_listxattr_nfs4_label() call which calls into the same
LSM hook, although that call is conditional on the server supporting
NFS_CAP_SECURITY_LABEL.  Based on a quick search, it appears the only
caller for nfs4_listxattr_nfs4_label() is nfs4_listxattr() so I'm
wondering if there isn't some room for improvement here.

I think there are two obvious options, and I'm curious about your
thoughts on which of these you would prefer, or if there is another
third option that you would like to see merged.

Option #1:
Essentially back out commit 243fea134633, removing the direct LSM call
in nfs4_listxattr() and relying on the nfs4_listxattr_nfs4_label() for
the LSM/SELinux xattrs.  I think we would want to remove the
NFS_CAP_SECURITY_LABEL check and build nfs4_listxattr_nfs4_label()
regardless of CONFIG_NFS_V4_SECURITY_LABEL.

Option #2:
Remove nfs4_listxattr_nfs4_label() entirely and keep the direct LSM
call in nfs4_listxattr(), with the required changes for this patch.

Thoughts?

[1] https://lore.kernel.org/all/20250425180921.86702-1-okorniev@redhat.com/ (local)

A gentle ping on the question above for the NFS folks.  If I don't
hear anything I'll hack up something and send it out for review, but I
thought it would nice if we could sort out the proper fix first.

Raising this thread back up again to see if the NFS folks have a
preference on option #1 or #2 above, or
something else altogether. Should returning of the security.selinux
xattr name from listxattr() be dependent on
NFS_CAP_SECURITY_LABEL being set by the server and should it be
dependent on CONFIG_NFS_V4_SECURITY_LABEL?

Thanks for bringing this back up Stephen, it would be good to get this resolved.

On second look, I realized that commit 243fea134633 ("NFSv4.2: fix
listxattr to return selinux security label") was likely motivated by
the same issue as commit 8b0ba61df5a1c44e2b3cf6 ("fs/xattr.c: fix
simple_xattr_list to always include security.* xattrs"), i.e. the
coreutils change that switched ls -Z from unconditionally calling
getxattr("security.selinux") (via libselinux getfilecon(3)) to only
doing so if listxattr() returns the "security.selinux" xattr name.
Hence, we want the call to security_inode_listsecurity() to be
unconditional, which favors option #2. My only residual question
though is that commit 243fea134633 put the call _after_ fetching the
user.* xattr names, whereas the nfs4_listxattr_nfs4_label() returns it
_before_ any user.* xattrs are appended. I'd be inclined to move up
the security_inode_listsecurity() call to replace the
nfs4_listxattr_nfs4_label() call along with option #2.

I've made an attempt to unify the two security_inode_listsecurity()
hook calls in the nfs4 code into a single, unconditional call from
nfs4_listxattr(), which can be found here:
https://lore.kernel.org/selinux/20251203195728.8592-1-stephen.smalley.work@gmail.com/T/#u (local)

If this is deemed acceptable by the NFS folks, then I can re-base this
patch on top of that one.