On 1/15/2024 10:18 AM, Roberto Sassu wrote: > From: Roberto Sassu <[email protected]> > > Since now IMA and EVM use their own integrity metadata, it is safe to > remove the 'integrity' LSM, with its management of integrity metadata. > > Keep the iint.c file only for loading IMA and EVM keys at boot, and for > creating the integrity directory in securityfs (we need to keep it for > retrocompatibility reasons). > > Signed-off-by: Roberto Sassu <[email protected]>
Reviewed-by: Casey Schaufler <[email protected]> > --- > include/linux/integrity.h | 14 --- > security/integrity/iint.c | 197 +-------------------------------- > security/integrity/integrity.h | 25 ----- > security/security.c | 2 - > 4 files changed, 2 insertions(+), 236 deletions(-) > > diff --git a/include/linux/integrity.h b/include/linux/integrity.h > index ef0f63ef5ebc..459b79683783 100644 > --- a/include/linux/integrity.h > +++ b/include/linux/integrity.h > @@ -19,24 +19,10 @@ enum integrity_status { > INTEGRITY_UNKNOWN, > }; > > -/* List of EVM protected security xattrs */ > #ifdef CONFIG_INTEGRITY > -extern struct integrity_iint_cache *integrity_inode_get(struct inode *inode); > -extern void integrity_inode_free(struct inode *inode); > extern void __init integrity_load_keys(void); > > #else > -static inline struct integrity_iint_cache * > - integrity_inode_get(struct inode *inode) > -{ > - return NULL; > -} > - > -static inline void integrity_inode_free(struct inode *inode) > -{ > - return; > -} > - > static inline void integrity_load_keys(void) > { > } > diff --git a/security/integrity/iint.c b/security/integrity/iint.c > index d4419a2a1e24..068ac6c2ae1e 100644 > --- a/security/integrity/iint.c > +++ b/security/integrity/iint.c > @@ -6,207 +6,14 @@ > * Mimi Zohar <[email protected]> > * > * File: integrity_iint.c > - * - implements the integrity hooks: integrity_inode_alloc, > - * integrity_inode_free > - * - cache integrity information associated with an inode > - * using a rbtree tree. > + * - initialize the integrity directory in securityfs > + * - load IMA and EVM keys > */ > -#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 __ro_after_init; > - > 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; > -} > - > -#define IMA_MAX_NESTING (FILESYSTEM_MAX_STACK_DEPTH+1) > - > -/* > - * It is not clear that IMA should be nested at all, but as long is it > measures > - * files both on overlayfs and on underlying fs, we need to annotate the iint > - * mutex to avoid lockdep false positives related to IMA + overlayfs. > - * See ovl_lockdep_annotate_inode_mutex_key() for more details. > - */ > -static inline void iint_lockdep_annotate(struct integrity_iint_cache *iint, > - struct inode *inode) > -{ > -#ifdef CONFIG_LOCKDEP > - static struct lock_class_key iint_mutex_key[IMA_MAX_NESTING]; > - > - int depth = inode->i_sb->s_stack_depth; > - > - if (WARN_ON_ONCE(depth < 0 || depth >= IMA_MAX_NESTING)) > - depth = 0; > - > - lockdep_set_class(&iint->mutex, &iint_mutex_key[depth]); > -#endif > -} > - > -static void iint_init_always(struct integrity_iint_cache *iint, > - struct inode *inode) > -{ > - iint->ima_hash = NULL; > - iint->version = 0; > - iint->flags = 0UL; > - iint->atomic_flags = 0UL; > - iint->ima_file_status = INTEGRITY_UNKNOWN; > - iint->ima_mmap_status = INTEGRITY_UNKNOWN; > - iint->ima_bprm_status = INTEGRITY_UNKNOWN; > - iint->ima_read_status = INTEGRITY_UNKNOWN; > - iint->ima_creds_status = INTEGRITY_UNKNOWN; > - iint->evm_status = INTEGRITY_UNKNOWN; > - iint->measured_pcrs = 0; > - mutex_init(&iint->mutex); > - iint_lockdep_annotate(iint, inode); > -} > - > -static void iint_free(struct integrity_iint_cache *iint) > -{ > - kfree(iint->ima_hash); > - mutex_destroy(&iint->mutex); > - kmem_cache_free(iint_cache, iint); > -} > - > -/** > - * integrity_inode_get - find or allocate an iint associated with an inode > - * @inode: pointer to the inode > - * @return: allocated iint > - * > - * Caller must lock i_mutex > - */ > -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; > - > - iint = integrity_iint_find(inode); > - if (iint) > - return iint; > - > - iint = kmem_cache_alloc(iint_cache, GFP_NOFS); > - if (!iint) > - return NULL; > - > - 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); > - > - write_unlock(&integrity_iint_lock); > - return iint; > -} > - > -/** > - * integrity_inode_free - called on security_inode_free > - * @inode: pointer to the inode > - * > - * Free the integrity information(iint) associated with an inode. > - */ > -void integrity_inode_free(struct inode *inode) > -{ > - struct integrity_iint_cache *iint; > - > - 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_free(iint); > -} > - > -static void iint_init_once(void *foo) > -{ > - struct integrity_iint_cache *iint = (struct integrity_iint_cache *) foo; > - > - memset(iint, 0, sizeof(*iint)); > -} > - > -static int __init integrity_iintcache_init(void) > -{ > - iint_cache = > - kmem_cache_create("iint_cache", sizeof(struct integrity_iint_cache), > - 0, SLAB_PANIC, iint_init_once); > - return 0; > -} > -DEFINE_LSM(integrity) = { > - .name = "integrity", > - .init = integrity_iintcache_init, > - .order = LSM_ORDER_LAST, > -}; > - > - > /* > * integrity_kernel_read - read data from the file > * > diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h > index 671fc50255f9..50d6f798e613 100644 > --- a/security/integrity/integrity.h > +++ b/security/integrity/integrity.h > @@ -102,31 +102,6 @@ struct ima_file_id { > __u8 hash[HASH_MAX_DIGESTSIZE]; > } __packed; > > -/* 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 */ > - unsigned long flags; > - unsigned long measured_pcrs; > - unsigned long atomic_flags; > - unsigned long real_ino; > - dev_t real_dev; > - enum integrity_status ima_file_status:4; > - enum integrity_status ima_mmap_status:4; > - enum integrity_status ima_bprm_status:4; > - enum integrity_status ima_read_status:4; > - enum integrity_status ima_creds_status:4; > - enum integrity_status evm_status:4; > - struct ima_digest_data *ima_hash; > -}; > - > -/* rbtree tree calls to lookup, insert, delete > - * integrity data associated with an inode. > - */ > -struct integrity_iint_cache *integrity_iint_find(struct inode *inode); > - > int integrity_kernel_read(struct file *file, loff_t offset, > void *addr, unsigned long count); > > diff --git a/security/security.c b/security/security.c > index f811cc376a7a..df87c0a7eaac 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -19,7 +19,6 @@ > #include <linux/kernel.h> > #include <linux/kernel_read_file.h> > #include <linux/lsm_hooks.h> > -#include <linux/integrity.h> > #include <linux/fsnotify.h> > #include <linux/mman.h> > #include <linux/mount.h> > @@ -1597,7 +1596,6 @@ static void inode_free_by_rcu(struct rcu_head *head) > */ > void security_inode_free(struct inode *inode) > { > - integrity_inode_free(inode); > call_void_hook(inode_free_security, inode); > /* > * The inode may still be referenced in a path walk and
