2023-09-02 07:47:03

by Casey Schaufler

[permalink] [raw]
Subject: Re: [PATCH v2 24/25] integrity: Move integrity functions to the LSM infrastructure

On 8/31/2023 4:38 AM, Roberto Sassu wrote:
> From: Roberto Sassu <[email protected]>
>
> Remove hardcoded calls to integrity functions from the LSM infrastructure.
> Also move the global declaration of integrity_inode_get() to
> security/integrity/integrity.h, so that the function can be still called by
> IMA.
>
> Register integrity functions as hook implementations in
> integrity_lsm_init().
>
> Signed-off-by: Roberto Sassu <[email protected]>

Reviewed-by: Casey Schaufler <[email protected]>

> ---
> include/linux/integrity.h | 26 --------------------------
> security/integrity/iint.c | 11 ++++++++++-
> security/integrity/integrity.h | 7 +++++++
> security/security.c | 9 +--------
> 4 files changed, 18 insertions(+), 35 deletions(-)
>
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index 2ea0f2f65ab6..afaae7ad26f4 100644
> --- a/include/linux/integrity.h
> +++ b/include/linux/integrity.h
> @@ -21,38 +21,12 @@ enum integrity_status {
>
> /* 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)
> {
> }
> #endif /* CONFIG_INTEGRITY */
>
> -#ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
> -
> -extern int integrity_kernel_module_request(char *kmod_name);
> -
> -#else
> -
> -static inline int integrity_kernel_module_request(char *kmod_name)
> -{
> - return 0;
> -}
> -
> -#endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
> -
> #endif /* _LINUX_INTEGRITY_H */
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index dd03f978b45c..70ee803a33ea 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -138,7 +138,7 @@ struct integrity_iint_cache *integrity_inode_get(struct inode *inode)
> *
> * Free the integrity information(iint) associated with an inode.
> */
> -void integrity_inode_free(struct inode *inode)
> +static void integrity_inode_free(struct inode *inode)
> {
> struct integrity_iint_cache *iint;
>
> @@ -167,12 +167,21 @@ static void init_once(void *foo)
> mutex_init(&iint->mutex);
> }
>
> +static struct security_hook_list integrity_hooks[] __ro_after_init = {
> + LSM_HOOK_INIT(inode_free_security, integrity_inode_free),
> +#ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
> + LSM_HOOK_INIT(kernel_module_request, integrity_kernel_module_request),
> +#endif
> +};
> +
> static int __init integrity_lsm_init(void)
> {
> iint_cache =
> kmem_cache_create("iint_cache", sizeof(struct integrity_iint_cache),
> 0, SLAB_PANIC, init_once);
>
> + security_add_hooks(integrity_hooks, ARRAY_SIZE(integrity_hooks),
> + "integrity");
> init_ima_lsm();
> init_evm_lsm();
> return 0;
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 83a465ac9013..e020c365997b 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -178,6 +178,7 @@ struct integrity_iint_cache {
> * integrity data associated with an inode.
> */
> struct integrity_iint_cache *integrity_iint_find(struct inode *inode);
> +struct integrity_iint_cache *integrity_inode_get(struct inode *inode);
>
> int integrity_kernel_read(struct file *file, loff_t offset,
> void *addr, unsigned long count);
> @@ -251,12 +252,18 @@ static inline int __init integrity_load_cert(const unsigned int id,
> #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
> int asymmetric_verify(struct key *keyring, const char *sig,
> int siglen, const char *data, int datalen);
> +int integrity_kernel_module_request(char *kmod_name);
> #else
> static inline int asymmetric_verify(struct key *keyring, const char *sig,
> int siglen, const char *data, int datalen)
> {
> return -EOPNOTSUPP;
> }
> +
> +static inline int integrity_kernel_module_request(char *kmod_name)
> +{
> + return 0;
> +}
> #endif
>
> #ifdef CONFIG_IMA_APPRAISE_MODSIG
> diff --git a/security/security.c b/security/security.c
> index 9ba36a8e5d65..e9275335aaa7 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>
> @@ -1497,7 +1496,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
> @@ -3090,12 +3088,7 @@ int security_kernel_create_files_as(struct cred *new, struct inode *inode)
> */
> int security_kernel_module_request(char *kmod_name)
> {
> - int ret;
> -
> - ret = call_int_hook(kernel_module_request, 0, kmod_name);
> - if (ret)
> - return ret;
> - return integrity_kernel_module_request(kmod_name);
> + return call_int_hook(kernel_module_request, 0, kmod_name);
> }
>
> /**