| Message ID | 20240115181809.885385-26-roberto.sassu@huaweicloud.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b6c0dec9f78bc691166d60347b52b3adf03d5875 |
| Headers | show |
| Series | security: Move IMA and EVM to the LSM infrastructure | expand |
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>
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