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(-)
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@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(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 970f28dbf253..6168a35cbd15 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8023,12 +8023,14 @@ static int nfs4_xattr_get_nfs4_label(const struct xattr_handler *handler,
static ssize_t
nfs4_listxattr_nfs4_label(struct inode *inode, char *list, size_t list_len)
{
- int len = 0;
+ ssize_t len = 0;
if (nfs_server_capable(inode, NFS_CAP_SECURITY_LABEL)) {
- len = security_inode_listsecurity(inode, list, list_len);
- if (len >= 0 && list_len && len > list_len)
- return -ERANGE;
+ ssize_t remaining = list_len;
+
+ len = security_inode_listsecurity(inode, &list, &remaining);
+ if (!len)
+ len = list_len - remaining;
}
return len;
}
diff --git a/fs/xattr.c b/fs/xattr.c
index 2fc314b27120..78387acab31b 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -492,9 +492,11 @@ vfs_listxattr(struct dentry *dentry, char *list, size_t size)
if (inode->i_op->listxattr) {
error = inode->i_op->listxattr(dentry, list, size);
} else {
- error = security_inode_listsecurity(inode, list, size);
- if (size && error > size)
- error = -ERANGE;
+ ssize_t remaining = size;
+
+ error = security_inode_listsecurity(inode, &list, &remaining);
+ if (!error)
+ error = size - remaining;
}
return error;
}
@@ -1469,17 +1471,10 @@ ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs,
if (err)
return err;
- err = security_inode_listsecurity(inode, buffer, remaining_size);
- if (err < 0)
+ err = security_inode_listsecurity(inode, &buffer, &remaining_size);
+ if (err)
return err;
- if (buffer) {
- if (remaining_size < err)
- return -ERANGE;
- buffer += err;
- }
- remaining_size -= err;
-
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);
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index bf3bbac4e02a..3c3919dcdebc 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -174,8 +174,8 @@ LSM_HOOK(int, -EOPNOTSUPP, inode_getsecurity, struct mnt_idmap *idmap,
struct inode *inode, const char *name, void **buffer, bool alloc)
LSM_HOOK(int, -EOPNOTSUPP, inode_setsecurity, struct inode *inode,
const char *name, const void *value, size_t size, int flags)
-LSM_HOOK(int, 0, inode_listsecurity, struct inode *inode, char *buffer,
- size_t buffer_size)
+LSM_HOOK(int, 0, inode_listsecurity, struct inode *inode, char **buffer,
+ ssize_t *remaining_size)
LSM_HOOK(void, LSM_RET_VOID, inode_getlsmprop, struct inode *inode,
struct lsm_prop *prop)
LSM_HOOK(int, 0, inode_copy_up, struct dentry *src, struct cred **new)
diff --git a/include/linux/security.h b/include/linux/security.h
index cc9b54d95d22..0efc6a0ab56d 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -457,7 +457,7 @@ int security_inode_getsecurity(struct mnt_idmap *idmap,
struct inode *inode, const char *name,
void **buffer, bool alloc);
int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags);
-int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size);
+int security_inode_listsecurity(struct inode *inode, char **buffer, ssize_t *remaining_size);
void security_inode_getlsmprop(struct inode *inode, struct lsm_prop *prop);
int security_inode_copy_up(struct dentry *src, struct cred **new);
int security_inode_copy_up_xattr(struct dentry *src, const char *name);
@@ -1077,7 +1077,8 @@ static inline int security_inode_setsecurity(struct inode *inode, const char *na
return -EOPNOTSUPP;
}
-static inline int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size)
+static inline int security_inode_listsecurity(struct inode *inode,
+ char **buffer, ssize_t *remaining_size)
{
return 0;
}
diff --git a/net/socket.c b/net/socket.c
index 9a0e720f0859..bbcaa3371fcd 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -560,17 +560,14 @@ static ssize_t sockfs_listxattr(struct dentry *dentry, char *buffer,
size_t size)
{
ssize_t len;
- ssize_t used = 0;
+ ssize_t used, remaining;
+ int err;
- len = security_inode_listsecurity(d_inode(dentry), buffer, size);
- if (len < 0)
- return len;
- used += len;
- if (buffer) {
- if (size < used)
- return -ERANGE;
- buffer += len;
- }
+ err = security_inode_listsecurity(d_inode(dentry), &buffer,
+ &remaining);
+ if (err)
+ return err;
+ used = size - remaining;
len = (XATTR_NAME_SOCKPROTONAME_LEN + 1);
used += len;
diff --git a/security/security.c b/security/security.c
index fb57e8fddd91..3985d040d5a9 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2710,22 +2710,22 @@ int security_inode_setsecurity(struct inode *inode, const char *name,
/**
* security_inode_listsecurity() - List the xattr security label names
* @inode: inode
- * @buffer: buffer
- * @buffer_size: size of buffer
+ * @buffer: pointer to buffer
+ * @remaining_size: pointer to remaining size of buffer
*
* Copy the extended attribute names for the security labels associated with
- * @inode into @buffer. The maximum size of @buffer is specified by
- * @buffer_size. @buffer may be NULL to request the size of the buffer
- * required.
+ * @inode into *(@buffer). The remaining size of @buffer is specified by
+ * *(@remaining_size). *(@buffer) may be NULL to request the size of the
+ * buffer required. Updates *(@buffer) and *(@remaining_size).
*
- * Return: Returns number of bytes used/required on success.
+ * Return: Returns 0 on success, or -errno on failure.
*/
int security_inode_listsecurity(struct inode *inode,
- char *buffer, size_t buffer_size)
+ char **buffer, ssize_t *remaining_size)
{
if (unlikely(IS_PRIVATE(inode)))
return 0;
- return call_int_hook(inode_listsecurity, inode, buffer, buffer_size);
+ return call_int_hook(inode_listsecurity, inode, buffer, remaining_size);
}
EXPORT_SYMBOL(security_inode_listsecurity);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index b8115df536ab..e6c98ebbf7bc 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3612,16 +3612,12 @@ static int selinux_inode_setsecurity(struct inode *inode, const char *name,
return 0;
}
-static int selinux_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size)
+static int selinux_inode_listsecurity(struct inode *inode, char **buffer,
+ ssize_t *remaining_size)
{
- const int len = sizeof(XATTR_NAME_SELINUX);
-
if (!selinux_initialized())
return 0;
-
- if (buffer && len <= buffer_size)
- memcpy(buffer, XATTR_NAME_SELINUX, len);
- return len;
+ return xattr_list_one(buffer, remaining_size, XATTR_NAME_SELINUX);
}
static void selinux_inode_getlsmprop(struct inode *inode, struct lsm_prop *prop)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 99833168604e..3f7ac865532e 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1619,17 +1619,12 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
* smack_inode_listsecurity - list the Smack attributes
* @inode: the object
* @buffer: where they go
- * @buffer_size: size of buffer
+ * @remaining_size: size of buffer
*/
-static int smack_inode_listsecurity(struct inode *inode, char *buffer,
- size_t buffer_size)
+static int smack_inode_listsecurity(struct inode *inode, char **buffer,
+ ssize_t *remaining_size)
{
- int len = sizeof(XATTR_NAME_SMACK);
-
- if (buffer != NULL && len <= buffer_size)
- memcpy(buffer, XATTR_NAME_SMACK, len);
-
- return len;
+ return xattr_list_one(buffer, remaining_size, XATTR_NAME_SMACK);
}
/**
--
2.49.0Stephen 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)?
--
Konstantin Andreev
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.
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.
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.
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
From: Stephen Smalley <stephen.smalley.work@gmail.com>
Date: Mon, 28 Apr 2025 15:50:19 -0400
> 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@gmail.com>
[...]
> diff --git a/net/socket.c b/net/socket.c
> index 9a0e720f0859..bbcaa3371fcd 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -560,17 +560,14 @@ static ssize_t sockfs_listxattr(struct dentry *dentry, char *buffer,
> size_t size)
> {
> ssize_t len;
> - ssize_t used = 0;
> + ssize_t used, remaining;
> + int err;
Paul: Could you sort this in the reverse xmas tree order before merging ?
https://docs.kernel.org/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs
otherwise the socket part looks good to me:
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
>
> - len = security_inode_listsecurity(d_inode(dentry), buffer, size);
> - if (len < 0)
> - return len;
> - used += len;
> - if (buffer) {
> - if (size < used)
> - return -ERANGE;
> - buffer += len;
> - }
> + err = security_inode_listsecurity(d_inode(dentry), &buffer,
> + &remaining);
> + if (err)
> + return err;
> + used = size - remaining;
>
> len = (XATTR_NAME_SOCKPROTONAME_LEN + 1);
> used += len;
On Tue, May 20, 2025 at 8:32 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
> From: Stephen Smalley <stephen.smalley.work@gmail.com>
> Date: Mon, 28 Apr 2025 15:50:19 -0400
> > 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@gmail.com>
> [...]
> > diff --git a/net/socket.c b/net/socket.c
> > index 9a0e720f0859..bbcaa3371fcd 100644
> > --- a/net/socket.c
> > +++ b/net/socket.c
> > @@ -560,17 +560,14 @@ static ssize_t sockfs_listxattr(struct dentry *dentry, char *buffer,
> > size_t size)
> > {
> > ssize_t len;
> > - ssize_t used = 0;
> > + ssize_t used, remaining;
> > + int err;
>
> Paul: Could you sort this in the reverse xmas tree order before merging ?
> https://docs.kernel.org/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs
Done, thanks for reviewing the patch.
> otherwise the socket part looks good to me:
>
> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
--
paul-moore.com
On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley <stephen.smalley.work@gmail.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@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. -- paul-moore.com
On Tue, Apr 29, 2025 at 7:35 PM Paul Moore <paul@paul-moore.com> wrote: > > On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley > <stephen.smalley.work@gmail.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@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. Note that this will need to have a conflict resolved with: https://lore.kernel.org/selinux/20250605164852.2016-1-stephen.smalley.work@gmail.com/ Fortunately it should be straightforward - just delete the line added by that patch since this patch fixes the security_inode_listsecurity() hook interface to return 0 or -errno itself. > > -- > paul-moore.com
On Thu, Jun 5, 2025 at 2:09 PM Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > On Tue, Apr 29, 2025 at 7:35 PM Paul Moore <paul@paul-moore.com> wrote: > > > > On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley > > <stephen.smalley.work@gmail.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@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. > > Note that this will need to have a conflict resolved with: > https://lore.kernel.org/selinux/20250605164852.2016-1-stephen.smalley.work@gmail.com/ > > Fortunately it should be straightforward - just delete the line added > by that patch since this patch fixes the security_inode_listsecurity() > hook interface to return 0 or -errno itself. Yep, I'm pretty much just waiting on -rc1 right now. -- paul-moore.com
On Tue, Apr 29, 2025 at 7:34 PM Paul Moore <paul@paul-moore.com> wrote: > On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley > <stephen.smalley.work@gmail.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@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. Link to the patch if you can't find it in your inbox: https://lore.kernel.org/linux-security-module/20250428195022.24587-2-stephen.smalley.work@gmail.com/ -- paul-moore.com
On 5/20/25 5:31 PM, Paul Moore wrote:
> On Tue, Apr 29, 2025 at 7:34 PM Paul Moore <paul@paul-moore.com> wrote:
>> On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley
>> <stephen.smalley.work@gmail.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@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 <anna.schumaker@oracle.com>
Anna
>
> Link to the patch if you can't find it in your inbox:
> https://lore.kernel.org/linux-security-module/20250428195022.24587-2-stephen.smalley.work@gmail.com/
>
> --
> paul-moore.com
On Tue, May 27, 2025 at 5:03 PM Anna Schumaker
<anna.schumaker@oracle.com> wrote:
> On 5/20/25 5:31 PM, Paul Moore wrote:
> > On Tue, Apr 29, 2025 at 7:34 PM Paul Moore <paul@paul-moore.com> wrote:
> >> On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley
> >> <stephen.smalley.work@gmail.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@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 <anna.schumaker@oracle.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/
--
paul-moore.com
On Thu, Jun 19, 2025 at 5:18 PM Paul Moore <paul@paul-moore.com> wrote:
> On Tue, May 27, 2025 at 5:03 PM Anna Schumaker
> <anna.schumaker@oracle.com> wrote:
> > On 5/20/25 5:31 PM, Paul Moore wrote:
> > > On Tue, Apr 29, 2025 at 7:34 PM Paul Moore <paul@paul-moore.com> wrote:
> > >> On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley
> > >> <stephen.smalley.work@gmail.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@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 <anna.schumaker@oracle.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
On Wed, Jul 23, 2025 at 10:10 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Thu, Jun 19, 2025 at 5:18 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Tue, May 27, 2025 at 5:03 PM Anna Schumaker
> > <anna.schumaker@oracle.com> wrote:
> > > On 5/20/25 5:31 PM, Paul Moore wrote:
> > > > On Tue, Apr 29, 2025 at 7:34 PM Paul Moore <paul@paul-moore.com> wrote:
> > > >> On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley
> > > >> <stephen.smalley.work@gmail.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@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 <anna.schumaker@oracle.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.
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?
On Wed, Dec 3, 2025 at 10:35 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Wed, Jul 23, 2025 at 10:10 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Thu, Jun 19, 2025 at 5:18 PM Paul Moore <paul@paul-moore.com> wrote:
> > > On Tue, May 27, 2025 at 5:03 PM Anna Schumaker
> > > <anna.schumaker@oracle.com> wrote:
> > > > On 5/20/25 5:31 PM, Paul Moore wrote:
> > > > > On Tue, Apr 29, 2025 at 7:34 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > >> On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley
> > > > >> <stephen.smalley.work@gmail.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@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 <anna.schumaker@oracle.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.
>
> 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.
--
paul-moore.com
On Wed, Dec 3, 2025 at 10:55 AM Paul Moore <paul@paul-moore.com> wrote:
>
> On Wed, Dec 3, 2025 at 10:35 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> > On Wed, Jul 23, 2025 at 10:10 PM Paul Moore <paul@paul-moore.com> wrote:
> > > On Thu, Jun 19, 2025 at 5:18 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > On Tue, May 27, 2025 at 5:03 PM Anna Schumaker
> > > > <anna.schumaker@oracle.com> wrote:
> > > > > On 5/20/25 5:31 PM, Paul Moore wrote:
> > > > > > On Tue, Apr 29, 2025 at 7:34 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > > >> On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley
> > > > > >> <stephen.smalley.work@gmail.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@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 <anna.schumaker@oracle.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.
> >
> > 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.
On Wed, Dec 3, 2025 at 1:08 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Wed, Dec 3, 2025 at 10:55 AM Paul Moore <paul@paul-moore.com> wrote:
> >
> > On Wed, Dec 3, 2025 at 10:35 AM Stephen Smalley
> > <stephen.smalley.work@gmail.com> wrote:
> > > On Wed, Jul 23, 2025 at 10:10 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > On Thu, Jun 19, 2025 at 5:18 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > > On Tue, May 27, 2025 at 5:03 PM Anna Schumaker
> > > > > <anna.schumaker@oracle.com> wrote:
> > > > > > On 5/20/25 5:31 PM, Paul Moore wrote:
> > > > > > > On Tue, Apr 29, 2025 at 7:34 PM Paul Moore <paul@paul-moore.com> wrote:
> > > > > > >> On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley
> > > > > > >> <stephen.smalley.work@gmail.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@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 <anna.schumaker@oracle.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.
> > >
> > > 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
If this is deemed acceptable by the NFS folks, then I can re-base this
patch on top of that one.
On 5/20/2025 2:31 PM, Paul Moore wrote: > On Tue, Apr 29, 2025 at 7:34 PM Paul Moore <paul@paul-moore.com> wrote: >> On Mon, Apr 28, 2025 at 4:15 PM Stephen Smalley >> <stephen.smalley.work@gmail.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@gmail.com> Acked-by: Casey Schaufler <casey@schaufler-ca.com> I haven't been able to test the change in Smack, but it appears reasonable. >>> --- >>> 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. > > Link to the patch if you can't find it in your inbox: > https://lore.kernel.org/linux-security-module/20250428195022.24587-2-stephen.smalley.work@gmail.com/ > > -- > paul-moore.com >
On Mon, Apr 28, 2025 at 03:50:19PM -0400, Stephen Smalley 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@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. It should be now. Thanks for doing this.
On Tue, Apr 29, 2025 at 3:46 AM Christian Brauner <brauner@kernel.org> wrote:
>
> On Mon, Apr 28, 2025 at 03:50:19PM -0400, Stephen Smalley 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@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.
>
> It should be now.
> Thanks for doing this.
Maybe I am looking in the wrong place?
$ git remote -v | grep vfs
vfs https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git (fetch)
vfs https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git (push)
$ git fetch vfs
$ git log vfs/vfs.fixes fs/xattr.c
commit f520bed25d17bb31c2d2d72b0a785b593a4e3179 (tag:
vfs-6.15-rc4.fixes, vfs/vfs.fixes, vfs.fixes)
Author: Jan Kara <jack@suse.cz>
Date: Thu Apr 24 15:22:47 2025 +0200
fs/xattr: Fix handling of AT_FDCWD in setxattrat(2) and getxattrat(2)
Currently, setxattrat(2) and getxattrat(2) are wrongly handling the
calls of the from setxattrat(AF_FDCWD, NULL, AT_EMPTY_PATH, ...) and
fail with -EBADF error instead of operating on CWD. Fix it.
Fixes: 6140be90ec70 ("fs/xattr: add *at family syscalls")
Signed-off-by: Jan Kara <jack@suse.cz>
Link: https://lore.kernel.org/20250424132246.16822-2-jack@suse.cz
Signed-off-by: Christian Brauner <brauner@kernel.org>
commit 46a7fcec097da5b3188dce608362fe6bf4ea26ee (tag: pull-xattr,
viro/work.xattr2)
Author: Colin Ian King <colin.i.king@gmail.com>
Date: Wed Oct 30 18:25:47 2024 +0000
xattr: remove redundant check on variable err
Curretly in function generic_listxattr the for_each_xattr_handler loop
checks err and will return out of the function if err is non-zero.
It's impossible for err to be non-zero at the end of the function where
err is checked again for a non-zero value. The final non-zero check is
therefore redundant and can be removed. Also move the declaration of
err into the loop.
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
© 2016 - 2025 Red Hat, Inc.