[v9,25/25] integrity: Remove LSM

  From: Roberto Sassu <roberto.sassu@huawei.com>

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 <roberto.sassu@huawei.com>
---
 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(-)
  

On 1/15/2024 10:18 AM, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> 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 <roberto.sassu@huawei.com>

Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>

> ---
>  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 <zohar@us.ibm.com>
>   *
>   * 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
  

On Jan 15, 2024 Roberto Sassu <roberto.sassu@huaweicloud.com> wrote:
> 
> 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 <roberto.sassu@huawei.com>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  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(-)

Acked-by: Paul Moore <paul@paul-moore.com>

--
paul-moore.com
  

On 1/15/24 13:18, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> 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 <roberto.sassu@huawei.com>

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>