diff mbox series

[v4,23/23] integrity: Switch from rbtree to LSM-managed blob for integrity_iint_cache

Message ID 20231027084234.485243-4-roberto.sassu@huaweicloud.com (mailing list archive)
State Superseded
Delegated to: Paul Moore
Headers show
Series security: Move IMA and EVM to the LSM infrastructure | expand

Commit Message

Roberto Sassu Oct. 27, 2023, 8:42 a.m. UTC 
From: Roberto Sassu <roberto.sassu@huawei.com>

Before the security field of kernel objects could be shared among LSMs with
the LSM stacking feature, IMA and EVM had to rely on an alternative storage
of inode metadata. The association between inode metadata and inode is
maintained through an rbtree.

Because of this alternative storage mechanism, there was no need to use
disjoint inode metadata, so IMA and EVM today still share them.

With the reservation mechanism offered by the LSM infrastructure, the
rbtree is no longer necessary, as each LSM could reserve a space in the
security blob for each inode. However, since IMA and EVM share the
inode metadata, they cannot directly reserve the space for them.

Instead, request from the 'integrity' LSM a space in the security blob for
the pointer of inode metadata (integrity_iint_cache structure). The other
reason for keeping the 'integrity' LSM is to preserve the original ordering
of IMA and EVM functions as when they were hardcoded.

Prefer reserving space for a pointer to allocating the integrity_iint_cache
structure directly, as IMA would require it only for a subset of inodes.
Always allocating it would cause a waste of memory.

Introduce two primitives for getting and setting the pointer of
integrity_iint_cache in the security blob, respectively
integrity_inode_get_iint() and integrity_inode_set_iint(). This would make
the code more understandable, as they directly replace rbtree operations.

Locking is not needed, as access to inode metadata is not shared, it is per
inode.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/integrity/iint.c      | 71 +++++-----------------------------
 security/integrity/integrity.h | 20 +++++++++-
 2 files changed, 29 insertions(+), 62 deletions(-)
diff mbox series

Patch

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 31a0fda3f1a1..0ce4b38c6b7d 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -14,56 +14,25 @@ 
 #include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/spinlock.h>
-#include <linux/rbtree.h>
 #include <linux/file.h>
 #include <linux/uaccess.h>
 #include <linux/security.h>
 #include <linux/lsm_hooks.h>
 #include "integrity.h"
 
-static struct rb_root integrity_iint_tree = RB_ROOT;
-static DEFINE_RWLOCK(integrity_iint_lock);
 static struct kmem_cache *iint_cache __read_mostly;
 
 struct dentry *integrity_dir;
 
-/*
- * __integrity_iint_find - return the iint associated with an inode
- */
-static struct integrity_iint_cache *__integrity_iint_find(struct inode *inode)
-{
-	struct integrity_iint_cache *iint;
-	struct rb_node *n = integrity_iint_tree.rb_node;
-
-	while (n) {
-		iint = rb_entry(n, struct integrity_iint_cache, rb_node);
-
-		if (inode < iint->inode)
-			n = n->rb_left;
-		else if (inode > iint->inode)
-			n = n->rb_right;
-		else
-			return iint;
-	}
-
-	return NULL;
-}
-
 /*
  * integrity_iint_find - return the iint associated with an inode
  */
 struct integrity_iint_cache *integrity_iint_find(struct inode *inode)
 {
-	struct integrity_iint_cache *iint;
-
 	if (!IS_IMA(inode))
 		return NULL;
 
-	read_lock(&integrity_iint_lock);
-	iint = __integrity_iint_find(inode);
-	read_unlock(&integrity_iint_lock);
-
-	return iint;
+	return integrity_inode_get_iint(inode);
 }
 
 #define IMA_MAX_NESTING (FILESYSTEM_MAX_STACK_DEPTH+1)
@@ -123,9 +92,7 @@  static void iint_free(struct integrity_iint_cache *iint)
  */
 struct integrity_iint_cache *integrity_inode_get(struct inode *inode)
 {
-	struct rb_node **p;
-	struct rb_node *node, *parent = NULL;
-	struct integrity_iint_cache *iint, *test_iint;
+	struct integrity_iint_cache *iint;
 
 	iint = integrity_iint_find(inode);
 	if (iint)
@@ -137,31 +104,10 @@  struct integrity_iint_cache *integrity_inode_get(struct inode *inode)
 
 	iint_init_always(iint, inode);
 
-	write_lock(&integrity_iint_lock);
-
-	p = &integrity_iint_tree.rb_node;
-	while (*p) {
-		parent = *p;
-		test_iint = rb_entry(parent, struct integrity_iint_cache,
-				     rb_node);
-		if (inode < test_iint->inode) {
-			p = &(*p)->rb_left;
-		} else if (inode > test_iint->inode) {
-			p = &(*p)->rb_right;
-		} else {
-			write_unlock(&integrity_iint_lock);
-			kmem_cache_free(iint_cache, iint);
-			return test_iint;
-		}
-	}
-
 	iint->inode = inode;
-	node = &iint->rb_node;
 	inode->i_flags |= S_IMA;
-	rb_link_node(node, parent, p);
-	rb_insert_color(node, &integrity_iint_tree);
+	integrity_inode_set_iint(inode, iint);
 
-	write_unlock(&integrity_iint_lock);
 	return iint;
 }
 
@@ -178,10 +124,8 @@  static void integrity_inode_free(struct inode *inode)
 	if (!IS_IMA(inode))
 		return;
 
-	write_lock(&integrity_iint_lock);
-	iint = __integrity_iint_find(inode);
-	rb_erase(&iint->rb_node, &integrity_iint_tree);
-	write_unlock(&integrity_iint_lock);
+	iint = integrity_iint_find(inode);
+	integrity_inode_set_iint(inode, NULL);
 
 	iint_free(iint);
 }
@@ -231,6 +175,10 @@  static int __init integrity_lsm_init(void)
 	return 0;
 }
 
+struct lsm_blob_sizes integrity_blob_sizes __ro_after_init = {
+	.lbs_inode = sizeof(struct integrity_iint_cache *),
+};
+
 /*
  * Keep it until IMA and EVM can use disjoint integrity metadata, and their
  * initialization order can be swapped without change in their behavior.
@@ -239,6 +187,7 @@  DEFINE_LSM(integrity) = {
 	.name = "integrity",
 	.init = integrity_lsm_init,
 	.order = LSM_ORDER_LAST,
+	.blobs = &integrity_blob_sizes,
 };
 
 /*
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index e4df82d6f6e7..ef2689b5264d 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -158,7 +158,6 @@  struct ima_file_id {
 
 /* integrity data associated with an inode */
 struct integrity_iint_cache {
-	struct rb_node rb_node;	/* rooted in integrity_iint_tree */
 	struct mutex mutex;	/* protects: version, flags, digest */
 	struct inode *inode;	/* back pointer to inode in question */
 	u64 version;		/* track inode changes */
@@ -192,6 +191,25 @@  int integrity_kernel_read(struct file *file, loff_t offset,
 #define INTEGRITY_KEYRING_MAX		4
 
 extern struct dentry *integrity_dir;
+extern struct lsm_blob_sizes integrity_blob_sizes;
+
+static inline struct integrity_iint_cache *
+integrity_inode_get_iint(const struct inode *inode)
+{
+	struct integrity_iint_cache **iint_sec;
+
+	iint_sec = inode->i_security + integrity_blob_sizes.lbs_inode;
+	return *iint_sec;
+}
+
+static inline void integrity_inode_set_iint(const struct inode *inode,
+					    struct integrity_iint_cache *iint)
+{
+	struct integrity_iint_cache **iint_sec;
+
+	iint_sec = inode->i_security + integrity_blob_sizes.lbs_inode;
+	*iint_sec = iint;
+}
 
 struct modsig;