[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHC9VhQnR6TKzzzpE9XQqiFivV0ECbVx7GH+1fQmz917-MAhsw@mail.gmail.com>
Date: Wed, 23 Jul 2025 22:09:56 -0400
From: Paul Moore <paul@...l-moore.com>
To: Anna Schumaker <anna.schumaker@...cle.com>
Cc: Trond Myklebust <trondmy@...nel.org>, Anna Schumaker <anna@...nel.org>,
Jakub Kicinski <kuba@...nel.org>, Casey Schaufler <casey@...aufler-ca.com>,
Stephen Smalley <stephen.smalley.work@...il.com>, Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, James Morris <jmorris@...ei.org>,
"Serge E. Hallyn" <serge@...lyn.com>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
Willem de Bruijn <willemb@...gle.com>, "David S. Miller" <davem@...emloft.net>,
Simon Horman <horms@...nel.org>, Ondrej Mosnacek <omosnace@...hat.com>, linux-nfs@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-security-module@...r.kernel.org, netdev@...r.kernel.org,
selinux@...r.kernel.org
Subject: Re: [PATCH v2] security,fs,nfs,net: update security_inode_listsecurity()
interface
On Thu, Jun 19, 2025 at 5:18 PM Paul Moore <paul@...l-moore.com> wrote:
> On Tue, May 27, 2025 at 5:03 PM Anna Schumaker
> <anna.schumaker@...cle.com> wrote:
> > On 5/20/25 5:31 PM, Paul Moore wrote:
> > > On Tue, Apr 29, 2025 at 7:34 PM Paul Moore <paul@...l-moore.com> wrote:
> > >> On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley
> > >> <stephen.smalley.work@...il.com> wrote:
> > >>>
> > >>> 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/
> > >>>
> > >>> Signed-off-by: Stephen Smalley <stephen.smalley.work@...il.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 <anna.schumaker@...cle.com>
>
> 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/
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.
--
paul-moore.com
Powered by blists - more mailing lists