Merge tag 'lsm-pr-20230829' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm

Pull LSM updates from Paul Moore:

 - Add proper multi-LSM support for xattrs in the
   security_inode_init_security() hook

   Historically the LSM layer has only allowed a single LSM to add an
   xattr to an inode, with IMA/EVM measuring that and adding its own as
   well. As we work towards promoting IMA/EVM to a "proper LSM" instead
   of the special case that it is now, we need to better support the
   case of multiple LSMs each adding xattrs to an inode and after
   several attempts we now appear to have something that is working
   well. It is worth noting that in the process of making this change we
   uncovered a problem with Smack's SMACK64TRANSMUTE xattr which is also
   fixed in this pull request.

 - Additional LSM hook constification

   Two patches to constify parameters to security_capget() and
   security_binder_transfer_file(). While I generally don't make a
   special note of who submitted these patches, these were the work of
   an Outreachy intern, Khadija Kamran, and that makes me happy;
   hopefully it does the same for all of you reading this.

 - LSM hook comment header fixes

   One patch to add a missing hook comment header, one to fix a minor
   typo.

 - Remove an old, unused credential function declaration

   It wasn't clear to me who should pick this up, but it was trivial,
   obviously correct, and arguably the LSM layer has a vested interest
   in credentials so I merged it. Sadly I'm now noticing that despite my
   subject line cleanup I didn't cleanup the "unsued" misspelling, sigh

* tag 'lsm-pr-20230829' of git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm:
  lsm: constify the 'file' parameter in security_binder_transfer_file()
  lsm: constify the 'target' parameter in security_capget()
  lsm: add comment block for security_sk_classify_flow LSM hook
  security: Fix ret values doc for security_inode_init_security()
  cred: remove unsued extern declaration change_create_files_as()
  evm: Support multiple LSMs providing an xattr
  evm: Align evm_inode_init_security() definition with LSM infrastructure
  smack: Set the SMACK64TRANSMUTE xattr in smack_inode_init_security()
  security: Allow all LSMs to provide xattrs for inode_init_security hook
  lsm: fix typo in security_file_lock() comment header

1 files changed, 61 insertions, 29 deletions

diff --git a/security/security.c b/security/security.c
index 3b454e9442b1..23b129d482a7 100644
--- a/security/security.c
+++ b/security/security.c
@@ -31,8 +31,6 @@
 #include <linux/msg.h>
 #include <net/flow.h>
 
-#define MAX_LSM_EVM_XATTR	2
-
 /* How many LSMs were built into the kernel? */
 #define LSM_COUNT (__end_lsm_info - __start_lsm_info)
 
@@ -212,6 +210,8 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
 	lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
 	lsm_set_blob_size(&needed->lbs_superblock, &blob_sizes.lbs_superblock);
 	lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
+	lsm_set_blob_size(&needed->lbs_xattr_count,
+			  &blob_sizes.lbs_xattr_count);
 }
 
 /* Prepare LSM for initialization. */
@@ -378,6 +378,7 @@ static void __init ordered_lsm_init(void)
 	init_debug("msg_msg blob size    = %d\n", blob_sizes.lbs_msg_msg);
 	init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
 	init_debug("task blob size       = %d\n", blob_sizes.lbs_task);
+	init_debug("xattr slots          = %d\n", blob_sizes.lbs_xattr_count);
 
 	/*
 	 * Create any kmem_caches needed for blobs
@@ -840,7 +841,7 @@ int security_binder_transfer_binder(const struct cred *from,
  * Return: Returns 0 if permission is granted.
  */
 int security_binder_transfer_file(const struct cred *from,
-				  const struct cred *to, struct file *file)
+				  const struct cred *to, const struct file *file)
 {
 	return call_int_hook(binder_transfer_file, 0, from, to, file);
 }
@@ -893,7 +894,7 @@ int security_ptrace_traceme(struct task_struct *parent)
  *
  * Return: Returns 0 if the capability sets were successfully obtained.
  */
-int security_capget(struct task_struct *target,
+int security_capget(const struct task_struct *target,
 		    kernel_cap_t *effective,
 		    kernel_cap_t *inheritable,
 		    kernel_cap_t *permitted)
@@ -1605,46 +1606,70 @@ EXPORT_SYMBOL(security_dentry_create_files_as);
  * created inode and set up the incore security field for the new inode.  This
  * hook is called by the fs code as part of the inode creation transaction and
  * provides for atomic labeling of the inode, unlike the post_create/mkdir/...
- * hooks called by the VFS.  The hook function is expected to allocate the name
- * and value via kmalloc, with the caller being responsible for calling kfree
- * after using them.  If the security module does not use security attributes
- * or does not wish to put a security attribute on this particular inode, then
- * it should return -EOPNOTSUPP to skip this processing.
+ * hooks called by the VFS.
+ *
+ * The hook function is expected to populate the xattrs array, by calling
+ * lsm_get_xattr_slot() to retrieve the slots reserved by the security module
+ * with the lbs_xattr_count field of the lsm_blob_sizes structure.  For each
+ * slot, the hook function should set ->name to the attribute name suffix
+ * (e.g. selinux), to allocate ->value (will be freed by the caller) and set it
+ * to the attribute value, to set ->value_len to the length of the value.  If
+ * the security module does not use security attributes or does not wish to put
+ * a security attribute on this particular inode, then it should return
+ * -EOPNOTSUPP to skip this processing.
  *
- * Return: Returns 0 on success, -EOPNOTSUPP if no security attribute is
- * needed, or -ENOMEM on memory allocation failure.
+ * Return: Returns 0 if the LSM successfully initialized all of the inode
+ *         security attributes that are required, negative values otherwise.
  */
 int security_inode_init_security(struct inode *inode, struct inode *dir,
 				 const struct qstr *qstr,
 				 const initxattrs initxattrs, void *fs_data)
 {
-	struct xattr new_xattrs[MAX_LSM_EVM_XATTR + 1];
-	struct xattr *lsm_xattr, *evm_xattr, *xattr;
-	int ret;
+	struct security_hook_list *hp;
+	struct xattr *new_xattrs = NULL;
+	int ret = -EOPNOTSUPP, xattr_count = 0;
 
 	if (unlikely(IS_PRIVATE(inode)))
 		return 0;
 
-	if (!initxattrs)
-		return call_int_hook(inode_init_security, -EOPNOTSUPP, inode,
-				     dir, qstr, NULL, NULL, NULL);
-	memset(new_xattrs, 0, sizeof(new_xattrs));
-	lsm_xattr = new_xattrs;
-	ret = call_int_hook(inode_init_security, -EOPNOTSUPP, inode, dir, qstr,
-			    &lsm_xattr->name,
-			    &lsm_xattr->value,
-			    &lsm_xattr->value_len);
-	if (ret)
+	if (!blob_sizes.lbs_xattr_count)
+		return 0;
+
+	if (initxattrs) {
+		/* Allocate +1 for EVM and +1 as terminator. */
+		new_xattrs = kcalloc(blob_sizes.lbs_xattr_count + 2,
+				     sizeof(*new_xattrs), GFP_NOFS);
+		if (!new_xattrs)
+			return -ENOMEM;
+	}
+
+	hlist_for_each_entry(hp, &security_hook_heads.inode_init_security,
+			     list) {
+		ret = hp->hook.inode_init_security(inode, dir, qstr, new_xattrs,
+						  &xattr_count);
+		if (ret && ret != -EOPNOTSUPP)
+			goto out;
+		/*
+		 * As documented in lsm_hooks.h, -EOPNOTSUPP in this context
+		 * means that the LSM is not willing to provide an xattr, not
+		 * that it wants to signal an error. Thus, continue to invoke
+		 * the remaining LSMs.
+		 */
+	}
+
+	/* If initxattrs() is NULL, xattr_count is zero, skip the call. */
+	if (!xattr_count)
 		goto out;
 
-	evm_xattr = lsm_xattr + 1;
-	ret = evm_inode_init_security(inode, lsm_xattr, evm_xattr);
+	ret = evm_inode_init_security(inode, dir, qstr, new_xattrs,
+				      &xattr_count);
 	if (ret)
 		goto out;
 	ret = initxattrs(inode, new_xattrs, fs_data);
 out:
-	for (xattr = new_xattrs; xattr->value != NULL; xattr++)
-		kfree(xattr->value);
+	for (; xattr_count > 0; xattr_count--)
+		kfree(new_xattrs[xattr_count - 1].value);
+	kfree(new_xattrs);
 	return (ret == -EOPNOTSUPP) ? 0 : ret;
 }
 EXPORT_SYMBOL(security_inode_init_security);
@@ -2731,7 +2756,7 @@ int security_file_lock(struct file *file, unsigned int cmd)
 /**
  * security_file_fcntl() - Check if fcntl() op is allowed
  * @file: file
- * @cmd: fnctl command
+ * @cmd: fcntl command
  * @arg: command argument
  *
  * Check permission before allowing the file operation specified by @cmd from
@@ -4410,6 +4435,13 @@ void security_sk_clone(const struct sock *sk, struct sock *newsk)
 }
 EXPORT_SYMBOL(security_sk_clone);
 
+/**
+ * security_sk_classify_flow() - Set a flow's secid based on socket
+ * @sk: original socket
+ * @flic: target flow
+ *
+ * Set the target flow's secid to socket's secid.
+ */
 void security_sk_classify_flow(const struct sock *sk, struct flowi_common *flic)
 {
 	call_void_hook(sk_getsecid, sk, &flic->flowic_secid);