Linux NFS development
 help / color / mirror / Atom feed
* [PATCH] security,fs,nfs,net: update security_inode_listsecurity() interface
@ 2025-04-28 15:55 Stephen Smalley
  2025-04-28 16:12 ` Stephen Smalley
  2025-04-28 16:17 ` Casey Schaufler
  0 siblings, 2 replies; 5+ messages in thread
From: Stephen Smalley @ 2025-04-28 15:55 UTC (permalink / raw)
  To: paul
  Cc: Stephen Smalley, Trond Myklebust, Anna Schumaker, Alexander Viro,
	Christian Brauner, Jan Kara, James Morris, Serge E. Hallyn,
	Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
	David S. Miller, Jakub Kicinski, Simon Horman, Ondrej Mosnacek,
	Casey Schaufler, linux-nfs, linux-kernel, linux-fsdevel,
	linux-security-module, netdev, selinux

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             |  9 +++++----
 fs/xattr.c                    | 20 ++++++++------------
 include/linux/lsm_hook_defs.h |  4 ++--
 include/linux/security.h      |  5 +++--
 net/socket.c                  |  8 +-------
 security/security.c           | 16 ++++++++--------
 security/selinux/hooks.c      | 10 +++-------
 security/smack/smack_lsm.c    | 13 ++++---------
 8 files changed, 34 insertions(+), 51 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 970f28dbf253..a1d7cb0acb5e 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -8023,12 +8023,13 @@ 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;
+	int err;
 
 	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;
+		err = security_inode_listsecurity(inode, &list, &len);
+		if (err)
+			len = err;
 	}
 	return len;
 }
diff --git a/fs/xattr.c b/fs/xattr.c
index 2fc314b27120..fdd2f387bfd5 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -492,9 +492,12 @@ 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;
+		char *buffer = list;
+		ssize_t len = 0;
+
+		error = security_inode_listsecurity(inode, &buffer, &len);
+		if (!error)
+			error = len;
 	}
 	return error;
 }
@@ -1469,17 +1472,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..52e3670dc89b 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -562,15 +562,9 @@ static ssize_t sockfs_listxattr(struct dentry *dentry, char *buffer,
 	ssize_t len;
 	ssize_t used = 0;
 
-	len = security_inode_listsecurity(d_inode(dentry), buffer, size);
+	len = security_inode_listsecurity(d_inode(dentry), &buffer, &used);
 	if (len < 0)
 		return len;
-	used += len;
-	if (buffer) {
-		if (size < used)
-			return -ERANGE;
-		buffer += len;
-	}
 
 	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.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] security,fs,nfs,net: update security_inode_listsecurity() interface
  2025-04-28 15:55 [PATCH] security,fs,nfs,net: update security_inode_listsecurity() interface Stephen Smalley
@ 2025-04-28 16:12 ` Stephen Smalley
  2025-04-28 16:17 ` Casey Schaufler
  1 sibling, 0 replies; 5+ messages in thread
From: Stephen Smalley @ 2025-04-28 16:12 UTC (permalink / raw)
  To: paul
  Cc: Trond Myklebust, Anna Schumaker, Alexander Viro,
	Christian Brauner, Jan Kara, James Morris, Serge E. Hallyn,
	Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
	David S. Miller, Jakub Kicinski, Simon Horman, Ondrej Mosnacek,
	Casey Schaufler, linux-nfs, linux-kernel, linux-fsdevel,
	linux-security-module, netdev, selinux

On Mon, Apr 28, 2025 at 12:00 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             |  9 +++++----
>  fs/xattr.c                    | 20 ++++++++------------
>  include/linux/lsm_hook_defs.h |  4 ++--
>  include/linux/security.h      |  5 +++--
>  net/socket.c                  |  8 +-------
>  security/security.c           | 16 ++++++++--------
>  security/selinux/hooks.c      | 10 +++-------
>  security/smack/smack_lsm.c    | 13 ++++---------
>  8 files changed, 34 insertions(+), 51 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 970f28dbf253..a1d7cb0acb5e 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -8023,12 +8023,13 @@ 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;
> +       int err;
>
>         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;
> +               err = security_inode_listsecurity(inode, &list, &len);
> +               if (err)
> +                       len = err;

That's obviously wrong, sorry for the noise. v2 coming.

>         }
>         return len;
>  }
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 2fc314b27120..fdd2f387bfd5 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -492,9 +492,12 @@ 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;
> +               char *buffer = list;
> +               ssize_t len = 0;
> +
> +               error = security_inode_listsecurity(inode, &buffer, &len);

Also wrong.

> +               if (!error)
> +                       error = len;
>         }
>         return error;
>  }
> @@ -1469,17 +1472,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..52e3670dc89b 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -562,15 +562,9 @@ static ssize_t sockfs_listxattr(struct dentry *dentry, char *buffer,
>         ssize_t len;
>         ssize_t used = 0;
>
> -       len = security_inode_listsecurity(d_inode(dentry), buffer, size);
> +       len = security_inode_listsecurity(d_inode(dentry), &buffer, &used);

And likewise wrong.

>         if (len < 0)
>                 return len;
> -       used += len;
> -       if (buffer) {
> -               if (size < used)
> -                       return -ERANGE;
> -               buffer += len;
> -       }
>
>         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.0
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] security,fs,nfs,net: update security_inode_listsecurity() interface
  2025-04-28 15:55 [PATCH] security,fs,nfs,net: update security_inode_listsecurity() interface Stephen Smalley
  2025-04-28 16:12 ` Stephen Smalley
@ 2025-04-28 16:17 ` Casey Schaufler
  2025-04-28 16:23   ` Stephen Smalley
  1 sibling, 1 reply; 5+ messages in thread
From: Casey Schaufler @ 2025-04-28 16:17 UTC (permalink / raw)
  To: Stephen Smalley, paul
  Cc: Trond Myklebust, Anna Schumaker, Alexander Viro,
	Christian Brauner, Jan Kara, James Morris, Serge E. Hallyn,
	Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
	David S. Miller, Jakub Kicinski, Simon Horman, Ondrej Mosnacek,
	linux-nfs, linux-kernel, linux-fsdevel, linux-security-module,
	netdev, selinux, Casey Schaufler

On 4/28/2025 8:55 AM, 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.
>
>  fs/nfs/nfs4proc.c             |  9 +++++----
>  fs/xattr.c                    | 20 ++++++++------------
>  include/linux/lsm_hook_defs.h |  4 ++--
>  include/linux/security.h      |  5 +++--
>  net/socket.c                  |  8 +-------
>  security/security.c           | 16 ++++++++--------
>  security/selinux/hooks.c      | 10 +++-------
>  security/smack/smack_lsm.c    | 13 ++++---------
>  8 files changed, 34 insertions(+), 51 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 970f28dbf253..a1d7cb0acb5e 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -8023,12 +8023,13 @@ 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;
> +	int err;
>  
>  	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;
> +		err = security_inode_listsecurity(inode, &list, &len);
> +		if (err)
> +			len = err;
>  	}
>  	return len;
>  }
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 2fc314b27120..fdd2f387bfd5 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -492,9 +492,12 @@ 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;
> +		char *buffer = list;
> +		ssize_t len = 0;
> +
> +		error = security_inode_listsecurity(inode, &buffer, &len);
> +		if (!error)
> +			error = len;
>  	}
>  	return error;
>  }
> @@ -1469,17 +1472,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)

How about "rem", "rsize" or some other name instead of the overly long
"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..52e3670dc89b 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -562,15 +562,9 @@ static ssize_t sockfs_listxattr(struct dentry *dentry, char *buffer,
>  	ssize_t len;
>  	ssize_t used = 0;
>  
> -	len = security_inode_listsecurity(d_inode(dentry), buffer, size);
> +	len = security_inode_listsecurity(d_inode(dentry), &buffer, &used);
>  	if (len < 0)
>  		return len;
> -	used += len;
> -	if (buffer) {
> -		if (size < used)
> -			return -ERANGE;
> -		buffer += len;
> -	}
>  
>  	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);
>  }
>  
>  /**

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] security,fs,nfs,net: update security_inode_listsecurity() interface
  2025-04-28 16:17 ` Casey Schaufler
@ 2025-04-28 16:23   ` Stephen Smalley
  2025-04-28 16:32     ` Casey Schaufler
  0 siblings, 1 reply; 5+ messages in thread
From: Stephen Smalley @ 2025-04-28 16:23 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: paul, Trond Myklebust, Anna Schumaker, Alexander Viro,
	Christian Brauner, Jan Kara, James Morris, Serge E. Hallyn,
	Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
	David S. Miller, Jakub Kicinski, Simon Horman, Ondrej Mosnacek,
	linux-nfs, linux-kernel, linux-fsdevel, linux-security-module,
	netdev, selinux

On Mon, Apr 28, 2025 at 12:17 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 4/28/2025 8:55 AM, 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.
>
> > 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)
>
> How about "rem", "rsize" or some other name instead of the overly long
> "remaining_size_"?

I don't especially care either way but was just being consistent with
the xattr_list_one() code.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] security,fs,nfs,net: update security_inode_listsecurity() interface
  2025-04-28 16:23   ` Stephen Smalley
@ 2025-04-28 16:32     ` Casey Schaufler
  0 siblings, 0 replies; 5+ messages in thread
From: Casey Schaufler @ 2025-04-28 16:32 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: paul, Trond Myklebust, Anna Schumaker, Alexander Viro,
	Christian Brauner, Jan Kara, James Morris, Serge E. Hallyn,
	Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
	David S. Miller, Jakub Kicinski, Simon Horman, Ondrej Mosnacek,
	linux-nfs, linux-kernel, linux-fsdevel, linux-security-module,
	netdev, selinux, Casey Schaufler

On 4/28/2025 9:23 AM, Stephen Smalley wrote:
> On Mon, Apr 28, 2025 at 12:17 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 4/28/2025 8:55 AM, 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.
>>> 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)
>> How about "rem", "rsize" or some other name instead of the overly long
>> "remaining_size_"?
> I don't especially care either way but was just being consistent with
> the xattr_list_one() code.

Sigh. Then I'd leave it as is.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-04-28 16:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-28 15:55 [PATCH] security,fs,nfs,net: update security_inode_listsecurity() interface Stephen Smalley
2025-04-28 16:12 ` Stephen Smalley
2025-04-28 16:17 ` Casey Schaufler
2025-04-28 16:23   ` Stephen Smalley
2025-04-28 16:32     ` Casey Schaufler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox