diff mbox series

[26/28] evm: Move to LSM infrastructure

Message ID	20230303182602.1088032-4-roberto.sassu@huaweicloud.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Roberto Sassu <roberto.sassu@huaweicloud.com> To: viro@zeniv.linux.org.uk, chuck.lever@oracle.com, jlayton@kernel.org, zohar@linux.ibm.com, dmitry.kasatkin@gmail.com, paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com, dhowells@redhat.com, jarkko@kernel.org, stephen.smalley.work@gmail.com, eparis@parisplace.org, casey@schaufler-ca.com, brauner@kernel.org Cc: linux-fsdevel@vger.kernel.org, linux-nfs@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, keyrings@vger.kernel.org, selinux@vger.kernel.org, linux-kernel@vger.kernel.org, stefanb@linux.ibm.com, Roberto Sassu <roberto.sassu@huawei.com> Subject: [PATCH 26/28] evm: Move to LSM infrastructure Date: Fri, 3 Mar 2023 19:26:00 +0100 Message-Id: <20230303182602.1088032-4-roberto.sassu@huaweicloud.com> In-Reply-To: <20230303181842.1087717-1-roberto.sassu@huaweicloud.com> References: <20230303181842.1087717-1-roberto.sassu@huaweicloud.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	security: Move IMA and EVM to the LSM infrastructure \| [00/28] security: Move IMA and EVM to the LSM infrastructure [01/28] ima: Align ima_inode_post_setattr() definition with LSM infrastructure [02/28] ima: Align ima_post_path_mknod() definition with LSM infrastructure [03/28] ima: Align ima_post_create_tmpfile() definition with LSM infrastructure [04/28] ima: Align ima_file_mprotect() definition with LSM infrastructure [05/28] ima: Align ima_inode_setxattr() definition with LSM infrastructure [06/28] ima: Align ima_inode_removexattr() definition with LSM infrastructure [07/28] ima: Align ima_post_read_file() definition with LSM infrastructure [08/28] evm: Align evm_inode_post_setattr() definition with LSM infrastructure [09/28] evm: Align evm_inode_setxattr() definition with LSM infrastructure [10/28] evm: Align evm_inode_post_setxattr() definition with LSM infrastructure [11/28] evm: Complete description of evm_inode_setattr() [12/28] fs: Fix description of vfs_tmpfile() [13/28] security: Align inode_setattr hook definition with EVM [14/28] security: Introduce inode_post_setattr hook [15/28] security: Introduce inode_post_removexattr hook [16/28] security: Introduce file_post_open hook [17/28] security: Introduce file_pre_free_security hook [18/28] security: Introduce path_post_mknod hook [19/28] security: Introduce inode_post_create_tmpfile hook [20/28] security: Introduce inode_post_set_acl hook [21/28] security: Introduce inode_post_remove_acl hook [22/28] security: Introduce key_post_create_or_update hook [23/28] security: Introduce LSM_ORDER_LAST [24/28] ima: Move to LSM infrastructure [25/28] ima: Move IMA-Appraisal to LSM infrastructure [26/28] evm: Move to LSM infrastructure [27/28] integrity: Move integrity functions to the LSM infrastructure [28/28] integrity: Switch from rbtree to LSM-managed blob for integrity_iint_cache

Commit Message

Roberto Sassu March 3, 2023, 6:26 p.m. UTC

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

As for IMA, remove hardcoded EVM function calls from the LSM infrastructure
and the VFS. Make EVM functions as static (except for
evm_inode_init_security(), which is exported), and register them as hook
implementations in init_evm_lsm(), called from integrity_lsm_init().

Finally, switch to the LSM reservation mechanism for the EVM xattr, by
setting the lbs_xattr field of the lsm_blob_sizes structure, and
consequently decrement the number of xattrs to allocate in
security_inode_init_security().

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/attr.c                         |   2 -
 fs/posix_acl.c                    |   3 -
 fs/xattr.c                        |   2 -
 include/linux/evm.h               | 116 ------------------------------
 security/integrity/evm/evm_main.c | 106 ++++++++++++++++++++++-----
 security/integrity/iint.c         |   7 ++
 security/integrity/integrity.h    |   9 +++
 security/security.c               |  41 +++--------
 8 files changed, 115 insertions(+), 171 deletions(-)

Comments

Casey Schaufler March 4, 2023, 9:36 p.m. UTC | #1

On 3/3/2023 10:26 AM, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> As for IMA, remove hardcoded EVM function calls from the LSM infrastructure
> and the VFS. Make EVM functions as static (except for
> evm_inode_init_security(), which is exported), and register them as hook
> implementations in init_evm_lsm(), called from integrity_lsm_init().
>
> Finally, switch to the LSM reservation mechanism for the EVM xattr, by
> setting the lbs_xattr field of the lsm_blob_sizes structure, and
> consequently decrement the number of xattrs to allocate in
> security_inode_init_security().
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  fs/attr.c                         |   2 -
>  fs/posix_acl.c                    |   3 -
>  fs/xattr.c                        |   2 -
>  include/linux/evm.h               | 116 ------------------------------
>  security/integrity/evm/evm_main.c | 106 ++++++++++++++++++++++-----
>  security/integrity/iint.c         |   7 ++
>  security/integrity/integrity.h    |   9 +++
>  security/security.c               |  41 +++--------
>  8 files changed, 115 insertions(+), 171 deletions(-)
>
> diff --git a/fs/attr.c b/fs/attr.c
> index 406d782dfab..1b911a627fe 100644
> --- a/fs/attr.c
> +++ b/fs/attr.c
> @@ -16,7 +16,6 @@
>  #include <linux/fcntl.h>
>  #include <linux/filelock.h>
>  #include <linux/security.h>
> -#include <linux/evm.h>
>  
>  #include "internal.h"
>  
> @@ -485,7 +484,6 @@ int notify_change(struct mnt_idmap *idmap, struct dentry *dentry,
>  	if (!error) {
>  		fsnotify_change(dentry, ia_valid);
>  		security_inode_post_setattr(idmap, dentry, ia_valid);
> -		evm_inode_post_setattr(idmap, dentry, ia_valid);
>  	}
>  
>  	return error;
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> index 5b8c92fce0c..608cb0a9f84 100644
> --- a/fs/posix_acl.c
> +++ b/fs/posix_acl.c
> @@ -26,7 +26,6 @@
>  #include <linux/mnt_idmapping.h>
>  #include <linux/iversion.h>
>  #include <linux/security.h>
> -#include <linux/evm.h>
>  #include <linux/fsnotify.h>
>  #include <linux/filelock.h>
>  
> @@ -1103,7 +1102,6 @@ int vfs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>  	if (!error) {
>  		fsnotify_xattr(dentry);
>  		security_inode_post_set_acl(dentry, acl_name, kacl);
> -		evm_inode_post_set_acl(dentry, acl_name, kacl);
>  	}
>  
>  out_inode_unlock:
> @@ -1214,7 +1212,6 @@ int vfs_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>  	if (!error) {
>  		fsnotify_xattr(dentry);
>  		security_inode_post_remove_acl(idmap, dentry, acl_name);
> -		evm_inode_post_remove_acl(idmap, dentry, acl_name);
>  	}
>  
>  out_inode_unlock:
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 10c959d9fc6..7708ffdacca 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -16,7 +16,6 @@
>  #include <linux/mount.h>
>  #include <linux/namei.h>
>  #include <linux/security.h>
> -#include <linux/evm.h>
>  #include <linux/syscalls.h>
>  #include <linux/export.h>
>  #include <linux/fsnotify.h>
> @@ -535,7 +534,6 @@ __vfs_removexattr_locked(struct mnt_idmap *idmap,
>  	if (!error) {
>  		fsnotify_xattr(dentry);
>  		security_inode_post_removexattr(dentry, name);
> -		evm_inode_post_removexattr(dentry, name);
>  	}
>  
>  out:
> diff --git a/include/linux/evm.h b/include/linux/evm.h
> index 8c043273552..61794299f09 100644
> --- a/include/linux/evm.h
> +++ b/include/linux/evm.h
> @@ -21,46 +21,6 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
>  					     void *xattr_value,
>  					     size_t xattr_value_len,
>  					     struct integrity_iint_cache *iint);
> -extern int evm_inode_setattr(struct mnt_idmap *idmap,
> -			     struct dentry *dentry, struct iattr *attr);
> -extern void evm_inode_post_setattr(struct mnt_idmap *idmap,
> -				   struct dentry *dentry, int ia_valid);
> -extern int evm_inode_setxattr(struct mnt_idmap *idmap,
> -			      struct dentry *dentry, const char *name,
> -			      const void *value, size_t size, int flags);
> -extern void evm_inode_post_setxattr(struct dentry *dentry,
> -				    const char *xattr_name,
> -				    const void *xattr_value,
> -				    size_t xattr_value_len,
> -				    int flags);
> -extern int evm_inode_removexattr(struct mnt_idmap *idmap,
> -				 struct dentry *dentry, const char *xattr_name);
> -extern void evm_inode_post_removexattr(struct dentry *dentry,
> -				       const char *xattr_name);
> -static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
> -					     struct dentry *dentry,
> -					     const char *acl_name)
> -{
> -	evm_inode_post_removexattr(dentry, acl_name);
> -}
> -extern int evm_inode_set_acl(struct mnt_idmap *idmap,
> -			     struct dentry *dentry, const char *acl_name,
> -			     struct posix_acl *kacl);
> -static inline int evm_inode_remove_acl(struct mnt_idmap *idmap,
> -				       struct dentry *dentry,
> -				       const char *acl_name)
> -{
> -	return evm_inode_set_acl(idmap, dentry, acl_name, NULL);
> -}
> -static inline void evm_inode_post_set_acl(struct dentry *dentry,
> -					  const char *acl_name,
> -					  struct posix_acl *kacl)
> -{
> -	return evm_inode_post_setxattr(dentry, acl_name, NULL, 0, 0);
> -}
> -extern int evm_inode_init_security(struct inode *inode, struct inode *dir,
> -				   const struct qstr *qstr,
> -				   struct xattr *xattrs);
>  extern bool evm_revalidate_status(const char *xattr_name);
>  extern int evm_protected_xattr_if_enabled(const char *req_xattr_name);
>  extern int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
> @@ -92,82 +52,6 @@ static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
>  }
>  #endif
>  
> -static inline int evm_inode_setattr(struct mnt_idmap *idmap,
> -				    struct dentry *dentry, struct iattr *attr)
> -{
> -	return 0;
> -}
> -
> -static inline void evm_inode_post_setattr(struct mnt_idmap *idmap,
> -					  struct dentry *dentry, int ia_valid)
> -{
> -	return;
> -}
> -
> -static inline int evm_inode_setxattr(struct mnt_idmap *idmap,
> -				     struct dentry *dentry, const char *name,
> -				     const void *value, size_t size, int flags)
> -{
> -	return 0;
> -}
> -
> -static inline void evm_inode_post_setxattr(struct dentry *dentry,
> -					   const char *xattr_name,
> -					   const void *xattr_value,
> -					   size_t xattr_value_len,
> -					   int flags)
> -{
> -	return;
> -}
> -
> -static inline int evm_inode_removexattr(struct mnt_idmap *idmap,
> -					struct dentry *dentry,
> -					const char *xattr_name)
> -{
> -	return 0;
> -}
> -
> -static inline void evm_inode_post_removexattr(struct dentry *dentry,
> -					      const char *xattr_name)
> -{
> -	return;
> -}
> -
> -static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
> -					     struct dentry *dentry,
> -					     const char *acl_name)
> -{
> -	return;
> -}
> -
> -static inline int evm_inode_set_acl(struct mnt_idmap *idmap,
> -				    struct dentry *dentry, const char *acl_name,
> -				    struct posix_acl *kacl)
> -{
> -	return 0;
> -}
> -
> -static inline int evm_inode_remove_acl(struct mnt_idmap *idmap,
> -				       struct dentry *dentry,
> -				       const char *acl_name)
> -{
> -	return 0;
> -}
> -
> -static inline void evm_inode_post_set_acl(struct dentry *dentry,
> -					  const char *acl_name,
> -					  struct posix_acl *kacl)
> -{
> -	return;
> -}
> -
> -static inline int evm_inode_init_security(struct inode *inode, struct inode *dir,
> -					  const struct qstr *qstr,
> -					  struct xattr *xattrs)
> -{
> -	return 0;
> -}
> -
>  static inline bool evm_revalidate_status(const char *xattr_name)
>  {
>  	return false;
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 8b5c472f78b..c45bc97277c 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -19,6 +19,7 @@
>  #include <linux/xattr.h>
>  #include <linux/integrity.h>
>  #include <linux/evm.h>
> +#include <linux/lsm_hooks.h>
>  #include <linux/magic.h>
>  #include <linux/posix_acl_xattr.h>
>  
> @@ -566,9 +567,9 @@ static int evm_protect_xattr(struct mnt_idmap *idmap,
>   * userspace from writing HMAC value.  Writing 'security.evm' requires
>   * requires CAP_SYS_ADMIN privileges.
>   */
> -int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
> -		       const char *xattr_name, const void *xattr_value,
> -		       size_t xattr_value_len, int flags)
> +static int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
> +			      const char *xattr_name, const void *xattr_value,
> +			      size_t xattr_value_len, int flags)
>  {
>  	const struct evm_ima_xattr_data *xattr_data = xattr_value;
>  
> @@ -598,8 +599,8 @@ int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
>   * Removing 'security.evm' requires CAP_SYS_ADMIN privileges and that
>   * the current value is valid.
>   */
> -int evm_inode_removexattr(struct mnt_idmap *idmap,
> -			  struct dentry *dentry, const char *xattr_name)
> +static int evm_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry,
> +				 const char *xattr_name)
>  {
>  	/* Policy permits modification of the protected xattrs even though
>  	 * there's no HMAC key loaded
> @@ -649,9 +650,11 @@ static inline int evm_inode_set_acl_change(struct mnt_idmap *idmap,
>   * Prevent modifying posix acls causing the EVM HMAC to be re-calculated
>   * and 'security.evm' xattr updated, unless the existing 'security.evm' is
>   * valid.
> + *
> + * Return: zero on success, -EPERM on failure.
>   */
> -int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> -		      const char *acl_name, struct posix_acl *kacl)
> +static int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> +			     const char *acl_name, struct posix_acl *kacl)
>  {
>  	enum integrity_status evm_status;
>  
> @@ -690,6 +693,24 @@ int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>  	return -EPERM;
>  }
>  
> +/**
> + * evm_inode_remove_acl - Protect the EVM extended attribute from posix acls
> + * @idmap: idmap of the mount
> + * @dentry: pointer to the affected dentry
> + * @acl_name: name of the posix acl
> + *
> + * Prevent removing posix acls causing the EVM HMAC to be re-calculated
> + * and 'security.evm' xattr updated, unless the existing 'security.evm' is
> + * valid.
> + *
> + * Return: zero on success, -EPERM on failure.
> + */
> +static int evm_inode_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> +				const char *acl_name)
> +{
> +	return evm_inode_set_acl(idmap, dentry, acl_name, NULL);
> +}
> +
>  static void evm_reset_status(struct inode *inode)
>  {
>  	struct integrity_iint_cache *iint;
> @@ -738,9 +759,11 @@ bool evm_revalidate_status(const char *xattr_name)
>   * __vfs_setxattr_noperm().  The caller of which has taken the inode's
>   * i_mutex lock.
>   */
> -void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
> -			     const void *xattr_value, size_t xattr_value_len,
> -			     int flags)
> +static void evm_inode_post_setxattr(struct dentry *dentry,
> +				    const char *xattr_name,
> +				    const void *xattr_value,
> +				    size_t xattr_value_len,
> +				    int flags)
>  {
>  	if (!evm_revalidate_status(xattr_name))
>  		return;
> @@ -756,6 +779,21 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
>  	evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len);
>  }
>  
> +/**
> + * evm_inode_post_set_acl - Update the EVM extended attribute from posix acls
> + * @dentry: pointer to the affected dentry
> + * @acl_name: name of the posix acl
> + * @kacl: pointer to the posix acls
> + *
> + * Update the 'security.evm' xattr with the EVM HMAC re-calculated after setting
> + * posix acls.
> + */
> +static void evm_inode_post_set_acl(struct dentry *dentry, const char *acl_name,
> +				   struct posix_acl *kacl)
> +{
> +	return evm_inode_post_setxattr(dentry, acl_name, NULL, 0, 0);
> +}
> +
>  /**
>   * evm_inode_post_removexattr - update 'security.evm' after removing the xattr
>   * @dentry: pointer to the affected dentry
> @@ -766,7 +804,8 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
>   * No need to take the i_mutex lock here, as this function is called from
>   * vfs_removexattr() which takes the i_mutex.
>   */
> -void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
> +static void evm_inode_post_removexattr(struct dentry *dentry,
> +				       const char *xattr_name)
>  {
>  	if (!evm_revalidate_status(xattr_name))
>  		return;
> @@ -782,6 +821,22 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
>  	evm_update_evmxattr(dentry, xattr_name, NULL, 0);
>  }
>  
> +/**
> + * evm_inode_post_remove_acl - Update the EVM extended attribute from posix acls
> + * @idmap: idmap of the mount
> + * @dentry: pointer to the affected dentry
> + * @acl_name: name of the posix acl
> + *
> + * Update the 'security.evm' xattr with the EVM HMAC re-calculated after
> + * removing posix acls.
> + */
> +static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
> +					     struct dentry *dentry,
> +					     const char *acl_name)
> +{
> +	evm_inode_post_removexattr(dentry, acl_name);
> +}
> +
>  static int evm_attr_change(struct mnt_idmap *idmap,
>  			   struct dentry *dentry, struct iattr *attr)
>  {
> @@ -805,8 +860,8 @@ static int evm_attr_change(struct mnt_idmap *idmap,
>   * Permit update of file attributes when files have a valid EVM signature,
>   * except in the case of them having an immutable portable signature.
>   */
> -int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> -		      struct iattr *attr)
> +static int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> +			     struct iattr *attr)
>  {
>  	unsigned int ia_valid = attr->ia_valid;
>  	enum integrity_status evm_status;
> @@ -853,8 +908,8 @@ int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>   * This function is called from notify_change(), which expects the caller
>   * to lock the inode's i_mutex.
>   */
> -void evm_inode_post_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> -			    int ia_valid)
> +static void evm_inode_post_setattr(struct mnt_idmap *idmap,
> +				   struct dentry *dentry, int ia_valid)
>  {
>  	if (!evm_revalidate_status(NULL))
>  		return;
> @@ -892,7 +947,7 @@ int evm_inode_init_security(struct inode *inode, struct inode *dir,
>  	if (!evm_protected_xattrs)
>  		return -EOPNOTSUPP;
>  
> -	evm_xattr = xattr;
> +	evm_xattr = xattrs + integrity_blob_sizes.lbs_xattr;

Please don't do this inline. Convention is to use a function,
intergrity_xattrs() for this.

>  
>  	xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);
>  	if (!xattr_data)
> @@ -952,4 +1007,23 @@ static int __init init_evm(void)
>  	return error;
>  }
>  
> +static struct security_hook_list evm_hooks[] __lsm_ro_after_init = {
> +	LSM_HOOK_INIT(inode_setattr, evm_inode_setattr),
> +	LSM_HOOK_INIT(inode_post_setattr, evm_inode_post_setattr),
> +	LSM_HOOK_INIT(inode_setxattr, evm_inode_setxattr),
> +	LSM_HOOK_INIT(inode_set_acl, evm_inode_set_acl),
> +	LSM_HOOK_INIT(inode_post_set_acl, evm_inode_post_set_acl),
> +	LSM_HOOK_INIT(inode_remove_acl, evm_inode_remove_acl),
> +	LSM_HOOK_INIT(inode_post_remove_acl, evm_inode_post_remove_acl),
> +	LSM_HOOK_INIT(inode_post_setxattr, evm_inode_post_setxattr),
> +	LSM_HOOK_INIT(inode_removexattr, evm_inode_removexattr),
> +	LSM_HOOK_INIT(inode_post_removexattr, evm_inode_post_removexattr),
> +	LSM_HOOK_INIT(inode_init_security, evm_inode_init_security),
> +};
> +
> +void __init init_evm_lsm(void)
> +{
> +	security_add_hooks(evm_hooks, ARRAY_SIZE(evm_hooks), "integrity");
> +}
> +
>  late_initcall(init_evm);
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index bbadf974b31..952d5ea4e18 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -179,12 +179,19 @@ static int __init integrity_lsm_init(void)
>  			      0, SLAB_PANIC, init_once);
>  
>  	init_ima_lsm();
> +	init_evm_lsm();
>  	return 0;
>  }
> +
> +struct lsm_blob_sizes integrity_blob_sizes __lsm_ro_after_init = {
> +	.lbs_xattr = 1,

Really? 1 byte? Don't even think of storing number of elements in lbs_xattr.
The linux_blob_size structure contains sizes of blobs, not number of elements.

> +};
> +
>  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 c72d375a356..76e7eda6651 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -188,6 +188,7 @@ 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;
>  
>  struct modsig;
>  
> @@ -199,6 +200,14 @@ static inline void __init init_ima_lsm(void)
>  }
>  #endif
>  
> +#ifdef CONFIG_EVM
> +void __init init_evm_lsm(void);
> +#else
> +static inline void __init init_evm_lsm(void)
> +{
> +}
> +#endif
> +
>  #ifdef CONFIG_INTEGRITY_SIGNATURE
>  
>  int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
> diff --git a/security/security.c b/security/security.c
> index 9bc6a4ef758..74abf04feef 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -20,13 +20,13 @@
>  #include <linux/kernel_read_file.h>
>  #include <linux/lsm_hooks.h>
>  #include <linux/integrity.h>
> -#include <linux/evm.h>
>  #include <linux/fsnotify.h>
>  #include <linux/mman.h>
>  #include <linux/mount.h>
>  #include <linux/personality.h>
>  #include <linux/backing-dev.h>
>  #include <linux/string.h>
> +#include <linux/xattr.h>
>  #include <linux/msg.h>
>  #include <net/flow.h>
>  
> @@ -1662,8 +1662,8 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
>  	if (!initxattrs)
>  		return call_int_hook(inode_init_security, -EOPNOTSUPP, inode,
>  				    dir, qstr, NULL);
> -	/* Allocate +1 for EVM and +1 as terminator. */
> -	new_xattrs = kcalloc(blob_sizes.lbs_xattr + 2, sizeof(*new_xattrs),
> +	/* Allocate +1 for terminator. */
> +	new_xattrs = kcalloc(blob_sizes.lbs_xattr + 1, sizeof(*new_xattrs),
>  			     GFP_NOFS);
>  	if (!new_xattrs)
>  		return -ENOMEM;
> @@ -1699,9 +1699,6 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
>  	if (!num_filled_xattrs)
>  		goto out;
>  
> -	ret = evm_inode_init_security(inode, dir, qstr, new_xattrs);
> -	if (ret && ret != -EOPNOTSUPP)
> -		goto out;
>  	ret = initxattrs(inode, new_xattrs, fs_data);
>  out:
>  	for (xattr = new_xattrs; xattr->value != NULL; xattr++)
> @@ -2201,14 +2198,9 @@ int security_inode_permission(struct inode *inode, int mask)
>  int security_inode_setattr(struct mnt_idmap *idmap,
>  			   struct dentry *dentry, struct iattr *attr)
>  {
> -	int ret;
> -
>  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>  		return 0;
> -	ret = call_int_hook(inode_setattr, 0, idmap, dentry, attr);
> -	if (ret)
> -		return ret;
> -	return evm_inode_setattr(idmap, dentry, attr);
> +	return call_int_hook(inode_setattr, 0, idmap, dentry, attr);
>  }
>  EXPORT_SYMBOL_GPL(security_inode_setattr);
>  
> @@ -2272,9 +2264,7 @@ int security_inode_setxattr(struct mnt_idmap *idmap,
>  
>  	if (ret == 1)
>  		ret = cap_inode_setxattr(dentry, name, value, size, flags);
> -	if (ret)
> -		return ret;
> -	return evm_inode_setxattr(idmap, dentry, name, value, size, flags);
> +	return ret;
>  }
>  
>  /**
> @@ -2293,15 +2283,10 @@ int security_inode_set_acl(struct mnt_idmap *idmap,
>  			   struct dentry *dentry, const char *acl_name,
>  			   struct posix_acl *kacl)
>  {
> -	int ret;
> -
>  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>  		return 0;
> -	ret = call_int_hook(inode_set_acl, 0, idmap, dentry, acl_name,
> -			    kacl);
> -	if (ret)
> -		return ret;
> -	return evm_inode_set_acl(idmap, dentry, acl_name, kacl);
> +	return call_int_hook(inode_set_acl, 0, idmap, dentry, acl_name,
> +			     kacl);
>  }
>  
>  /**
> @@ -2354,14 +2339,9 @@ int security_inode_get_acl(struct mnt_idmap *idmap,
>  int security_inode_remove_acl(struct mnt_idmap *idmap,
>  			      struct dentry *dentry, const char *acl_name)
>  {
> -	int ret;
> -
>  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>  		return 0;
> -	ret = call_int_hook(inode_remove_acl, 0, idmap, dentry, acl_name);
> -	if (ret)
> -		return ret;
> -	return evm_inode_remove_acl(idmap, dentry, acl_name);
> +	return call_int_hook(inode_remove_acl, 0, idmap, dentry, acl_name);
>  }
>  
>  /**
> @@ -2397,7 +2377,6 @@ void security_inode_post_setxattr(struct dentry *dentry, const char *name,
>  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>  		return;
>  	call_void_hook(inode_post_setxattr, dentry, name, value, size, flags);
> -	evm_inode_post_setxattr(dentry, name, value, size, flags);
>  }
>  
>  /**
> @@ -2458,9 +2437,7 @@ int security_inode_removexattr(struct mnt_idmap *idmap,
>  	ret = call_int_hook(inode_removexattr, 1, idmap, dentry, name);
>  	if (ret == 1)
>  		ret = cap_inode_removexattr(idmap, dentry, name);
> -	if (ret)
> -		return ret;
> -	return evm_inode_removexattr(idmap, dentry, name);
> +	return ret;
>  }
>  
>  /**

Roberto Sassu March 6, 2023, 9:21 a.m. UTC | #2

On Sat, 2023-03-04 at 13:36 -0800, Casey Schaufler wrote:
> On 3/3/2023 10:26 AM, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> > 
> > As for IMA, remove hardcoded EVM function calls from the LSM infrastructure
> > and the VFS. Make EVM functions as static (except for
> > evm_inode_init_security(), which is exported), and register them as hook
> > implementations in init_evm_lsm(), called from integrity_lsm_init().
> > 
> > Finally, switch to the LSM reservation mechanism for the EVM xattr, by
> > setting the lbs_xattr field of the lsm_blob_sizes structure, and
> > consequently decrement the number of xattrs to allocate in
> > security_inode_init_security().
> > 
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > ---
> >  fs/attr.c                         |   2 -
> >  fs/posix_acl.c                    |   3 -
> >  fs/xattr.c                        |   2 -
> >  include/linux/evm.h               | 116 ------------------------------
> >  security/integrity/evm/evm_main.c | 106 ++++++++++++++++++++++-----
> >  security/integrity/iint.c         |   7 ++
> >  security/integrity/integrity.h    |   9 +++
> >  security/security.c               |  41 +++--------
> >  8 files changed, 115 insertions(+), 171 deletions(-)
> > 
> > diff --git a/fs/attr.c b/fs/attr.c
> > index 406d782dfab..1b911a627fe 100644
> > --- a/fs/attr.c
> > +++ b/fs/attr.c
> > @@ -16,7 +16,6 @@
> >  #include <linux/fcntl.h>
> >  #include <linux/filelock.h>
> >  #include <linux/security.h>
> > -#include <linux/evm.h>
> >  
> >  #include "internal.h"
> >  
> > @@ -485,7 +484,6 @@ int notify_change(struct mnt_idmap *idmap, struct dentry *dentry,
> >  	if (!error) {
> >  		fsnotify_change(dentry, ia_valid);
> >  		security_inode_post_setattr(idmap, dentry, ia_valid);
> > -		evm_inode_post_setattr(idmap, dentry, ia_valid);
> >  	}
> >  
> >  	return error;
> > diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> > index 5b8c92fce0c..608cb0a9f84 100644
> > --- a/fs/posix_acl.c
> > +++ b/fs/posix_acl.c
> > @@ -26,7 +26,6 @@
> >  #include <linux/mnt_idmapping.h>
> >  #include <linux/iversion.h>
> >  #include <linux/security.h>
> > -#include <linux/evm.h>
> >  #include <linux/fsnotify.h>
> >  #include <linux/filelock.h>
> >  
> > @@ -1103,7 +1102,6 @@ int vfs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> >  	if (!error) {
> >  		fsnotify_xattr(dentry);
> >  		security_inode_post_set_acl(dentry, acl_name, kacl);
> > -		evm_inode_post_set_acl(dentry, acl_name, kacl);
> >  	}
> >  
> >  out_inode_unlock:
> > @@ -1214,7 +1212,6 @@ int vfs_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> >  	if (!error) {
> >  		fsnotify_xattr(dentry);
> >  		security_inode_post_remove_acl(idmap, dentry, acl_name);
> > -		evm_inode_post_remove_acl(idmap, dentry, acl_name);
> >  	}
> >  
> >  out_inode_unlock:
> > diff --git a/fs/xattr.c b/fs/xattr.c
> > index 10c959d9fc6..7708ffdacca 100644
> > --- a/fs/xattr.c
> > +++ b/fs/xattr.c
> > @@ -16,7 +16,6 @@
> >  #include <linux/mount.h>
> >  #include <linux/namei.h>
> >  #include <linux/security.h>
> > -#include <linux/evm.h>
> >  #include <linux/syscalls.h>
> >  #include <linux/export.h>
> >  #include <linux/fsnotify.h>
> > @@ -535,7 +534,6 @@ __vfs_removexattr_locked(struct mnt_idmap *idmap,
> >  	if (!error) {
> >  		fsnotify_xattr(dentry);
> >  		security_inode_post_removexattr(dentry, name);
> > -		evm_inode_post_removexattr(dentry, name);
> >  	}
> >  
> >  out:
> > diff --git a/include/linux/evm.h b/include/linux/evm.h
> > index 8c043273552..61794299f09 100644
> > --- a/include/linux/evm.h
> > +++ b/include/linux/evm.h
> > @@ -21,46 +21,6 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
> >  					     void *xattr_value,
> >  					     size_t xattr_value_len,
> >  					     struct integrity_iint_cache *iint);
> > -extern int evm_inode_setattr(struct mnt_idmap *idmap,
> > -			     struct dentry *dentry, struct iattr *attr);
> > -extern void evm_inode_post_setattr(struct mnt_idmap *idmap,
> > -				   struct dentry *dentry, int ia_valid);
> > -extern int evm_inode_setxattr(struct mnt_idmap *idmap,
> > -			      struct dentry *dentry, const char *name,
> > -			      const void *value, size_t size, int flags);
> > -extern void evm_inode_post_setxattr(struct dentry *dentry,
> > -				    const char *xattr_name,
> > -				    const void *xattr_value,
> > -				    size_t xattr_value_len,
> > -				    int flags);
> > -extern int evm_inode_removexattr(struct mnt_idmap *idmap,
> > -				 struct dentry *dentry, const char *xattr_name);
> > -extern void evm_inode_post_removexattr(struct dentry *dentry,
> > -				       const char *xattr_name);
> > -static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
> > -					     struct dentry *dentry,
> > -					     const char *acl_name)
> > -{
> > -	evm_inode_post_removexattr(dentry, acl_name);
> > -}
> > -extern int evm_inode_set_acl(struct mnt_idmap *idmap,
> > -			     struct dentry *dentry, const char *acl_name,
> > -			     struct posix_acl *kacl);
> > -static inline int evm_inode_remove_acl(struct mnt_idmap *idmap,
> > -				       struct dentry *dentry,
> > -				       const char *acl_name)
> > -{
> > -	return evm_inode_set_acl(idmap, dentry, acl_name, NULL);
> > -}
> > -static inline void evm_inode_post_set_acl(struct dentry *dentry,
> > -					  const char *acl_name,
> > -					  struct posix_acl *kacl)
> > -{
> > -	return evm_inode_post_setxattr(dentry, acl_name, NULL, 0, 0);
> > -}
> > -extern int evm_inode_init_security(struct inode *inode, struct inode *dir,
> > -				   const struct qstr *qstr,
> > -				   struct xattr *xattrs);
> >  extern bool evm_revalidate_status(const char *xattr_name);
> >  extern int evm_protected_xattr_if_enabled(const char *req_xattr_name);
> >  extern int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
> > @@ -92,82 +52,6 @@ static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
> >  }
> >  #endif
> >  
> > -static inline int evm_inode_setattr(struct mnt_idmap *idmap,
> > -				    struct dentry *dentry, struct iattr *attr)
> > -{
> > -	return 0;
> > -}
> > -
> > -static inline void evm_inode_post_setattr(struct mnt_idmap *idmap,
> > -					  struct dentry *dentry, int ia_valid)
> > -{
> > -	return;
> > -}
> > -
> > -static inline int evm_inode_setxattr(struct mnt_idmap *idmap,
> > -				     struct dentry *dentry, const char *name,
> > -				     const void *value, size_t size, int flags)
> > -{
> > -	return 0;
> > -}
> > -
> > -static inline void evm_inode_post_setxattr(struct dentry *dentry,
> > -					   const char *xattr_name,
> > -					   const void *xattr_value,
> > -					   size_t xattr_value_len,
> > -					   int flags)
> > -{
> > -	return;
> > -}
> > -
> > -static inline int evm_inode_removexattr(struct mnt_idmap *idmap,
> > -					struct dentry *dentry,
> > -					const char *xattr_name)
> > -{
> > -	return 0;
> > -}
> > -
> > -static inline void evm_inode_post_removexattr(struct dentry *dentry,
> > -					      const char *xattr_name)
> > -{
> > -	return;
> > -}
> > -
> > -static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
> > -					     struct dentry *dentry,
> > -					     const char *acl_name)
> > -{
> > -	return;
> > -}
> > -
> > -static inline int evm_inode_set_acl(struct mnt_idmap *idmap,
> > -				    struct dentry *dentry, const char *acl_name,
> > -				    struct posix_acl *kacl)
> > -{
> > -	return 0;
> > -}
> > -
> > -static inline int evm_inode_remove_acl(struct mnt_idmap *idmap,
> > -				       struct dentry *dentry,
> > -				       const char *acl_name)
> > -{
> > -	return 0;
> > -}
> > -
> > -static inline void evm_inode_post_set_acl(struct dentry *dentry,
> > -					  const char *acl_name,
> > -					  struct posix_acl *kacl)
> > -{
> > -	return;
> > -}
> > -
> > -static inline int evm_inode_init_security(struct inode *inode, struct inode *dir,
> > -					  const struct qstr *qstr,
> > -					  struct xattr *xattrs)
> > -{
> > -	return 0;
> > -}
> > -
> >  static inline bool evm_revalidate_status(const char *xattr_name)
> >  {
> >  	return false;
> > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > index 8b5c472f78b..c45bc97277c 100644
> > --- a/security/integrity/evm/evm_main.c
> > +++ b/security/integrity/evm/evm_main.c
> > @@ -19,6 +19,7 @@
> >  #include <linux/xattr.h>
> >  #include <linux/integrity.h>
> >  #include <linux/evm.h>
> > +#include <linux/lsm_hooks.h>
> >  #include <linux/magic.h>
> >  #include <linux/posix_acl_xattr.h>
> >  
> > @@ -566,9 +567,9 @@ static int evm_protect_xattr(struct mnt_idmap *idmap,
> >   * userspace from writing HMAC value.  Writing 'security.evm' requires
> >   * requires CAP_SYS_ADMIN privileges.
> >   */
> > -int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > -		       const char *xattr_name, const void *xattr_value,
> > -		       size_t xattr_value_len, int flags)
> > +static int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > +			      const char *xattr_name, const void *xattr_value,
> > +			      size_t xattr_value_len, int flags)
> >  {
> >  	const struct evm_ima_xattr_data *xattr_data = xattr_value;
> >  
> > @@ -598,8 +599,8 @@ int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
> >   * Removing 'security.evm' requires CAP_SYS_ADMIN privileges and that
> >   * the current value is valid.
> >   */
> > -int evm_inode_removexattr(struct mnt_idmap *idmap,
> > -			  struct dentry *dentry, const char *xattr_name)
> > +static int evm_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > +				 const char *xattr_name)
> >  {
> >  	/* Policy permits modification of the protected xattrs even though
> >  	 * there's no HMAC key loaded
> > @@ -649,9 +650,11 @@ static inline int evm_inode_set_acl_change(struct mnt_idmap *idmap,
> >   * Prevent modifying posix acls causing the EVM HMAC to be re-calculated
> >   * and 'security.evm' xattr updated, unless the existing 'security.evm' is
> >   * valid.
> > + *
> > + * Return: zero on success, -EPERM on failure.
> >   */
> > -int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> > -		      const char *acl_name, struct posix_acl *kacl)
> > +static int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> > +			     const char *acl_name, struct posix_acl *kacl)
> >  {
> >  	enum integrity_status evm_status;
> >  
> > @@ -690,6 +693,24 @@ int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> >  	return -EPERM;
> >  }
> >  
> > +/**
> > + * evm_inode_remove_acl - Protect the EVM extended attribute from posix acls
> > + * @idmap: idmap of the mount
> > + * @dentry: pointer to the affected dentry
> > + * @acl_name: name of the posix acl
> > + *
> > + * Prevent removing posix acls causing the EVM HMAC to be re-calculated
> > + * and 'security.evm' xattr updated, unless the existing 'security.evm' is
> > + * valid.
> > + *
> > + * Return: zero on success, -EPERM on failure.
> > + */
> > +static int evm_inode_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> > +				const char *acl_name)
> > +{
> > +	return evm_inode_set_acl(idmap, dentry, acl_name, NULL);
> > +}
> > +
> >  static void evm_reset_status(struct inode *inode)
> >  {
> >  	struct integrity_iint_cache *iint;
> > @@ -738,9 +759,11 @@ bool evm_revalidate_status(const char *xattr_name)
> >   * __vfs_setxattr_noperm().  The caller of which has taken the inode's
> >   * i_mutex lock.
> >   */
> > -void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
> > -			     const void *xattr_value, size_t xattr_value_len,
> > -			     int flags)
> > +static void evm_inode_post_setxattr(struct dentry *dentry,
> > +				    const char *xattr_name,
> > +				    const void *xattr_value,
> > +				    size_t xattr_value_len,
> > +				    int flags)
> >  {
> >  	if (!evm_revalidate_status(xattr_name))
> >  		return;
> > @@ -756,6 +779,21 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
> >  	evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len);
> >  }
> >  
> > +/**
> > + * evm_inode_post_set_acl - Update the EVM extended attribute from posix acls
> > + * @dentry: pointer to the affected dentry
> > + * @acl_name: name of the posix acl
> > + * @kacl: pointer to the posix acls
> > + *
> > + * Update the 'security.evm' xattr with the EVM HMAC re-calculated after setting
> > + * posix acls.
> > + */
> > +static void evm_inode_post_set_acl(struct dentry *dentry, const char *acl_name,
> > +				   struct posix_acl *kacl)
> > +{
> > +	return evm_inode_post_setxattr(dentry, acl_name, NULL, 0, 0);
> > +}
> > +
> >  /**
> >   * evm_inode_post_removexattr - update 'security.evm' after removing the xattr
> >   * @dentry: pointer to the affected dentry
> > @@ -766,7 +804,8 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
> >   * No need to take the i_mutex lock here, as this function is called from
> >   * vfs_removexattr() which takes the i_mutex.
> >   */
> > -void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
> > +static void evm_inode_post_removexattr(struct dentry *dentry,
> > +				       const char *xattr_name)
> >  {
> >  	if (!evm_revalidate_status(xattr_name))
> >  		return;
> > @@ -782,6 +821,22 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
> >  	evm_update_evmxattr(dentry, xattr_name, NULL, 0);
> >  }
> >  
> > +/**
> > + * evm_inode_post_remove_acl - Update the EVM extended attribute from posix acls
> > + * @idmap: idmap of the mount
> > + * @dentry: pointer to the affected dentry
> > + * @acl_name: name of the posix acl
> > + *
> > + * Update the 'security.evm' xattr with the EVM HMAC re-calculated after
> > + * removing posix acls.
> > + */
> > +static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
> > +					     struct dentry *dentry,
> > +					     const char *acl_name)
> > +{
> > +	evm_inode_post_removexattr(dentry, acl_name);
> > +}
> > +
> >  static int evm_attr_change(struct mnt_idmap *idmap,
> >  			   struct dentry *dentry, struct iattr *attr)
> >  {
> > @@ -805,8 +860,8 @@ static int evm_attr_change(struct mnt_idmap *idmap,
> >   * Permit update of file attributes when files have a valid EVM signature,
> >   * except in the case of them having an immutable portable signature.
> >   */
> > -int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > -		      struct iattr *attr)
> > +static int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > +			     struct iattr *attr)
> >  {
> >  	unsigned int ia_valid = attr->ia_valid;
> >  	enum integrity_status evm_status;
> > @@ -853,8 +908,8 @@ int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> >   * This function is called from notify_change(), which expects the caller
> >   * to lock the inode's i_mutex.
> >   */
> > -void evm_inode_post_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > -			    int ia_valid)
> > +static void evm_inode_post_setattr(struct mnt_idmap *idmap,
> > +				   struct dentry *dentry, int ia_valid)
> >  {
> >  	if (!evm_revalidate_status(NULL))
> >  		return;
> > @@ -892,7 +947,7 @@ int evm_inode_init_security(struct inode *inode, struct inode *dir,
> >  	if (!evm_protected_xattrs)
> >  		return -EOPNOTSUPP;
> >  
> > -	evm_xattr = xattr;
> > +	evm_xattr = xattrs + integrity_blob_sizes.lbs_xattr;
> 
> Please don't do this inline. Convention is to use a function,
> intergrity_xattrs() for this.

Ok.

> >  
> >  	xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);
> >  	if (!xattr_data)
> > @@ -952,4 +1007,23 @@ static int __init init_evm(void)
> >  	return error;
> >  }
> >  
> > +static struct security_hook_list evm_hooks[] __lsm_ro_after_init = {
> > +	LSM_HOOK_INIT(inode_setattr, evm_inode_setattr),
> > +	LSM_HOOK_INIT(inode_post_setattr, evm_inode_post_setattr),
> > +	LSM_HOOK_INIT(inode_setxattr, evm_inode_setxattr),
> > +	LSM_HOOK_INIT(inode_set_acl, evm_inode_set_acl),
> > +	LSM_HOOK_INIT(inode_post_set_acl, evm_inode_post_set_acl),
> > +	LSM_HOOK_INIT(inode_remove_acl, evm_inode_remove_acl),
> > +	LSM_HOOK_INIT(inode_post_remove_acl, evm_inode_post_remove_acl),
> > +	LSM_HOOK_INIT(inode_post_setxattr, evm_inode_post_setxattr),
> > +	LSM_HOOK_INIT(inode_removexattr, evm_inode_removexattr),
> > +	LSM_HOOK_INIT(inode_post_removexattr, evm_inode_post_removexattr),
> > +	LSM_HOOK_INIT(inode_init_security, evm_inode_init_security),
> > +};
> > +
> > +void __init init_evm_lsm(void)
> > +{
> > +	security_add_hooks(evm_hooks, ARRAY_SIZE(evm_hooks), "integrity");
> > +}
> > +
> >  late_initcall(init_evm);
> > diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> > index bbadf974b31..952d5ea4e18 100644
> > --- a/security/integrity/iint.c
> > +++ b/security/integrity/iint.c
> > @@ -179,12 +179,19 @@ static int __init integrity_lsm_init(void)
> >  			      0, SLAB_PANIC, init_once);
> >  
> >  	init_ima_lsm();
> > +	init_evm_lsm();
> >  	return 0;
> >  }
> > +
> > +struct lsm_blob_sizes integrity_blob_sizes __lsm_ro_after_init = {
> > +	.lbs_xattr = 1,
> 
> Really? 1 byte? Don't even think of storing number of elements in lbs_xattr.
> The linux_blob_size structure contains sizes of blobs, not number of elements.

Oh, I see it can be confusing.

However, lbs_xattr does not help to position in the security blob but
in the new_xattrs array, allocated in security_inode_init_security()
(see below). Any suggestion on how to make this part better?

Thanks

Roberto

> > +};
> > +
> >  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 c72d375a356..76e7eda6651 100644
> > --- a/security/integrity/integrity.h
> > +++ b/security/integrity/integrity.h
> > @@ -188,6 +188,7 @@ 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;
> >  
> >  struct modsig;
> >  
> > @@ -199,6 +200,14 @@ static inline void __init init_ima_lsm(void)
> >  }
> >  #endif
> >  
> > +#ifdef CONFIG_EVM
> > +void __init init_evm_lsm(void);
> > +#else
> > +static inline void __init init_evm_lsm(void)
> > +{
> > +}
> > +#endif
> > +
> >  #ifdef CONFIG_INTEGRITY_SIGNATURE
> >  
> >  int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
> > diff --git a/security/security.c b/security/security.c
> > index 9bc6a4ef758..74abf04feef 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -20,13 +20,13 @@
> >  #include <linux/kernel_read_file.h>
> >  #include <linux/lsm_hooks.h>
> >  #include <linux/integrity.h>
> > -#include <linux/evm.h>
> >  #include <linux/fsnotify.h>
> >  #include <linux/mman.h>
> >  #include <linux/mount.h>
> >  #include <linux/personality.h>
> >  #include <linux/backing-dev.h>
> >  #include <linux/string.h>
> > +#include <linux/xattr.h>
> >  #include <linux/msg.h>
> >  #include <net/flow.h>
> >  
> > @@ -1662,8 +1662,8 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> >  	if (!initxattrs)
> >  		return call_int_hook(inode_init_security, -EOPNOTSUPP, inode,
> >  				    dir, qstr, NULL);
> > -	/* Allocate +1 for EVM and +1 as terminator. */
> > -	new_xattrs = kcalloc(blob_sizes.lbs_xattr + 2, sizeof(*new_xattrs),
> > +	/* Allocate +1 for terminator. */
> > +	new_xattrs = kcalloc(blob_sizes.lbs_xattr + 1, sizeof(*new_xattrs),
> >  			     GFP_NOFS);
> >  	if (!new_xattrs)
> >  		return -ENOMEM;
> > @@ -1699,9 +1699,6 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> >  	if (!num_filled_xattrs)
> >  		goto out;
> >  
> > -	ret = evm_inode_init_security(inode, dir, qstr, new_xattrs);
> > -	if (ret && ret != -EOPNOTSUPP)
> > -		goto out;
> >  	ret = initxattrs(inode, new_xattrs, fs_data);
> >  out:
> >  	for (xattr = new_xattrs; xattr->value != NULL; xattr++)
> > @@ -2201,14 +2198,9 @@ int security_inode_permission(struct inode *inode, int mask)
> >  int security_inode_setattr(struct mnt_idmap *idmap,
> >  			   struct dentry *dentry, struct iattr *attr)
> >  {
> > -	int ret;
> > -
> >  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> >  		return 0;
> > -	ret = call_int_hook(inode_setattr, 0, idmap, dentry, attr);
> > -	if (ret)
> > -		return ret;
> > -	return evm_inode_setattr(idmap, dentry, attr);
> > +	return call_int_hook(inode_setattr, 0, idmap, dentry, attr);
> >  }
> >  EXPORT_SYMBOL_GPL(security_inode_setattr);
> >  
> > @@ -2272,9 +2264,7 @@ int security_inode_setxattr(struct mnt_idmap *idmap,
> >  
> >  	if (ret == 1)
> >  		ret = cap_inode_setxattr(dentry, name, value, size, flags);
> > -	if (ret)
> > -		return ret;
> > -	return evm_inode_setxattr(idmap, dentry, name, value, size, flags);
> > +	return ret;
> >  }
> >  
> >  /**
> > @@ -2293,15 +2283,10 @@ int security_inode_set_acl(struct mnt_idmap *idmap,
> >  			   struct dentry *dentry, const char *acl_name,
> >  			   struct posix_acl *kacl)
> >  {
> > -	int ret;
> > -
> >  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> >  		return 0;
> > -	ret = call_int_hook(inode_set_acl, 0, idmap, dentry, acl_name,
> > -			    kacl);
> > -	if (ret)
> > -		return ret;
> > -	return evm_inode_set_acl(idmap, dentry, acl_name, kacl);
> > +	return call_int_hook(inode_set_acl, 0, idmap, dentry, acl_name,
> > +			     kacl);
> >  }
> >  
> >  /**
> > @@ -2354,14 +2339,9 @@ int security_inode_get_acl(struct mnt_idmap *idmap,
> >  int security_inode_remove_acl(struct mnt_idmap *idmap,
> >  			      struct dentry *dentry, const char *acl_name)
> >  {
> > -	int ret;
> > -
> >  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> >  		return 0;
> > -	ret = call_int_hook(inode_remove_acl, 0, idmap, dentry, acl_name);
> > -	if (ret)
> > -		return ret;
> > -	return evm_inode_remove_acl(idmap, dentry, acl_name);
> > +	return call_int_hook(inode_remove_acl, 0, idmap, dentry, acl_name);
> >  }
> >  
> >  /**
> > @@ -2397,7 +2377,6 @@ void security_inode_post_setxattr(struct dentry *dentry, const char *name,
> >  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> >  		return;
> >  	call_void_hook(inode_post_setxattr, dentry, name, value, size, flags);
> > -	evm_inode_post_setxattr(dentry, name, value, size, flags);
> >  }
> >  
> >  /**
> > @@ -2458,9 +2437,7 @@ int security_inode_removexattr(struct mnt_idmap *idmap,
> >  	ret = call_int_hook(inode_removexattr, 1, idmap, dentry, name);
> >  	if (ret == 1)
> >  		ret = cap_inode_removexattr(idmap, dentry, name);
> > -	if (ret)
> > -		return ret;
> > -	return evm_inode_removexattr(idmap, dentry, name);
> > +	return ret;
> >  }
> >  
> >  /**

Casey Schaufler March 7, 2023, 4:54 p.m. UTC | #3

On 3/6/2023 1:21 AM, Roberto Sassu wrote:
> On Sat, 2023-03-04 at 13:36 -0800, Casey Schaufler wrote:
>> On 3/3/2023 10:26 AM, Roberto Sassu wrote:
>>> From: Roberto Sassu <roberto.sassu@huawei.com>
>>>
>>> As for IMA, remove hardcoded EVM function calls from the LSM infrastructure
>>> and the VFS. Make EVM functions as static (except for
>>> evm_inode_init_security(), which is exported), and register them as hook
>>> implementations in init_evm_lsm(), called from integrity_lsm_init().
>>>
>>> Finally, switch to the LSM reservation mechanism for the EVM xattr, by
>>> setting the lbs_xattr field of the lsm_blob_sizes structure, and
>>> consequently decrement the number of xattrs to allocate in
>>> security_inode_init_security().
>>>
>>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>>> ---
>>>  fs/attr.c                         |   2 -
>>>  fs/posix_acl.c                    |   3 -
>>>  fs/xattr.c                        |   2 -
>>>  include/linux/evm.h               | 116 ------------------------------
>>>  security/integrity/evm/evm_main.c | 106 ++++++++++++++++++++++-----
>>>  security/integrity/iint.c         |   7 ++
>>>  security/integrity/integrity.h    |   9 +++
>>>  security/security.c               |  41 +++--------
>>>  8 files changed, 115 insertions(+), 171 deletions(-)
>>>
>>> diff --git a/fs/attr.c b/fs/attr.c
>>> index 406d782dfab..1b911a627fe 100644
>>> --- a/fs/attr.c
>>> +++ b/fs/attr.c
>>> @@ -16,7 +16,6 @@
>>>  #include <linux/fcntl.h>
>>>  #include <linux/filelock.h>
>>>  #include <linux/security.h>
>>> -#include <linux/evm.h>
>>>  
>>>  #include "internal.h"
>>>  
>>> @@ -485,7 +484,6 @@ int notify_change(struct mnt_idmap *idmap, struct dentry *dentry,
>>>  	if (!error) {
>>>  		fsnotify_change(dentry, ia_valid);
>>>  		security_inode_post_setattr(idmap, dentry, ia_valid);
>>> -		evm_inode_post_setattr(idmap, dentry, ia_valid);
>>>  	}
>>>  
>>>  	return error;
>>> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
>>> index 5b8c92fce0c..608cb0a9f84 100644
>>> --- a/fs/posix_acl.c
>>> +++ b/fs/posix_acl.c
>>> @@ -26,7 +26,6 @@
>>>  #include <linux/mnt_idmapping.h>
>>>  #include <linux/iversion.h>
>>>  #include <linux/security.h>
>>> -#include <linux/evm.h>
>>>  #include <linux/fsnotify.h>
>>>  #include <linux/filelock.h>
>>>  
>>> @@ -1103,7 +1102,6 @@ int vfs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>>>  	if (!error) {
>>>  		fsnotify_xattr(dentry);
>>>  		security_inode_post_set_acl(dentry, acl_name, kacl);
>>> -		evm_inode_post_set_acl(dentry, acl_name, kacl);
>>>  	}
>>>  
>>>  out_inode_unlock:
>>> @@ -1214,7 +1212,6 @@ int vfs_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>>>  	if (!error) {
>>>  		fsnotify_xattr(dentry);
>>>  		security_inode_post_remove_acl(idmap, dentry, acl_name);
>>> -		evm_inode_post_remove_acl(idmap, dentry, acl_name);
>>>  	}
>>>  
>>>  out_inode_unlock:
>>> diff --git a/fs/xattr.c b/fs/xattr.c
>>> index 10c959d9fc6..7708ffdacca 100644
>>> --- a/fs/xattr.c
>>> +++ b/fs/xattr.c
>>> @@ -16,7 +16,6 @@
>>>  #include <linux/mount.h>
>>>  #include <linux/namei.h>
>>>  #include <linux/security.h>
>>> -#include <linux/evm.h>
>>>  #include <linux/syscalls.h>
>>>  #include <linux/export.h>
>>>  #include <linux/fsnotify.h>
>>> @@ -535,7 +534,6 @@ __vfs_removexattr_locked(struct mnt_idmap *idmap,
>>>  	if (!error) {
>>>  		fsnotify_xattr(dentry);
>>>  		security_inode_post_removexattr(dentry, name);
>>> -		evm_inode_post_removexattr(dentry, name);
>>>  	}
>>>  
>>>  out:
>>> diff --git a/include/linux/evm.h b/include/linux/evm.h
>>> index 8c043273552..61794299f09 100644
>>> --- a/include/linux/evm.h
>>> +++ b/include/linux/evm.h
>>> @@ -21,46 +21,6 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
>>>  					     void *xattr_value,
>>>  					     size_t xattr_value_len,
>>>  					     struct integrity_iint_cache *iint);
>>> -extern int evm_inode_setattr(struct mnt_idmap *idmap,
>>> -			     struct dentry *dentry, struct iattr *attr);
>>> -extern void evm_inode_post_setattr(struct mnt_idmap *idmap,
>>> -				   struct dentry *dentry, int ia_valid);
>>> -extern int evm_inode_setxattr(struct mnt_idmap *idmap,
>>> -			      struct dentry *dentry, const char *name,
>>> -			      const void *value, size_t size, int flags);
>>> -extern void evm_inode_post_setxattr(struct dentry *dentry,
>>> -				    const char *xattr_name,
>>> -				    const void *xattr_value,
>>> -				    size_t xattr_value_len,
>>> -				    int flags);
>>> -extern int evm_inode_removexattr(struct mnt_idmap *idmap,
>>> -				 struct dentry *dentry, const char *xattr_name);
>>> -extern void evm_inode_post_removexattr(struct dentry *dentry,
>>> -				       const char *xattr_name);
>>> -static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
>>> -					     struct dentry *dentry,
>>> -					     const char *acl_name)
>>> -{
>>> -	evm_inode_post_removexattr(dentry, acl_name);
>>> -}
>>> -extern int evm_inode_set_acl(struct mnt_idmap *idmap,
>>> -			     struct dentry *dentry, const char *acl_name,
>>> -			     struct posix_acl *kacl);
>>> -static inline int evm_inode_remove_acl(struct mnt_idmap *idmap,
>>> -				       struct dentry *dentry,
>>> -				       const char *acl_name)
>>> -{
>>> -	return evm_inode_set_acl(idmap, dentry, acl_name, NULL);
>>> -}
>>> -static inline void evm_inode_post_set_acl(struct dentry *dentry,
>>> -					  const char *acl_name,
>>> -					  struct posix_acl *kacl)
>>> -{
>>> -	return evm_inode_post_setxattr(dentry, acl_name, NULL, 0, 0);
>>> -}
>>> -extern int evm_inode_init_security(struct inode *inode, struct inode *dir,
>>> -				   const struct qstr *qstr,
>>> -				   struct xattr *xattrs);
>>>  extern bool evm_revalidate_status(const char *xattr_name);
>>>  extern int evm_protected_xattr_if_enabled(const char *req_xattr_name);
>>>  extern int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
>>> @@ -92,82 +52,6 @@ static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
>>>  }
>>>  #endif
>>>  
>>> -static inline int evm_inode_setattr(struct mnt_idmap *idmap,
>>> -				    struct dentry *dentry, struct iattr *attr)
>>> -{
>>> -	return 0;
>>> -}
>>> -
>>> -static inline void evm_inode_post_setattr(struct mnt_idmap *idmap,
>>> -					  struct dentry *dentry, int ia_valid)
>>> -{
>>> -	return;
>>> -}
>>> -
>>> -static inline int evm_inode_setxattr(struct mnt_idmap *idmap,
>>> -				     struct dentry *dentry, const char *name,
>>> -				     const void *value, size_t size, int flags)
>>> -{
>>> -	return 0;
>>> -}
>>> -
>>> -static inline void evm_inode_post_setxattr(struct dentry *dentry,
>>> -					   const char *xattr_name,
>>> -					   const void *xattr_value,
>>> -					   size_t xattr_value_len,
>>> -					   int flags)
>>> -{
>>> -	return;
>>> -}
>>> -
>>> -static inline int evm_inode_removexattr(struct mnt_idmap *idmap,
>>> -					struct dentry *dentry,
>>> -					const char *xattr_name)
>>> -{
>>> -	return 0;
>>> -}
>>> -
>>> -static inline void evm_inode_post_removexattr(struct dentry *dentry,
>>> -					      const char *xattr_name)
>>> -{
>>> -	return;
>>> -}
>>> -
>>> -static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
>>> -					     struct dentry *dentry,
>>> -					     const char *acl_name)
>>> -{
>>> -	return;
>>> -}
>>> -
>>> -static inline int evm_inode_set_acl(struct mnt_idmap *idmap,
>>> -				    struct dentry *dentry, const char *acl_name,
>>> -				    struct posix_acl *kacl)
>>> -{
>>> -	return 0;
>>> -}
>>> -
>>> -static inline int evm_inode_remove_acl(struct mnt_idmap *idmap,
>>> -				       struct dentry *dentry,
>>> -				       const char *acl_name)
>>> -{
>>> -	return 0;
>>> -}
>>> -
>>> -static inline void evm_inode_post_set_acl(struct dentry *dentry,
>>> -					  const char *acl_name,
>>> -					  struct posix_acl *kacl)
>>> -{
>>> -	return;
>>> -}
>>> -
>>> -static inline int evm_inode_init_security(struct inode *inode, struct inode *dir,
>>> -					  const struct qstr *qstr,
>>> -					  struct xattr *xattrs)
>>> -{
>>> -	return 0;
>>> -}
>>> -
>>>  static inline bool evm_revalidate_status(const char *xattr_name)
>>>  {
>>>  	return false;
>>> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
>>> index 8b5c472f78b..c45bc97277c 100644
>>> --- a/security/integrity/evm/evm_main.c
>>> +++ b/security/integrity/evm/evm_main.c
>>> @@ -19,6 +19,7 @@
>>>  #include <linux/xattr.h>
>>>  #include <linux/integrity.h>
>>>  #include <linux/evm.h>
>>> +#include <linux/lsm_hooks.h>
>>>  #include <linux/magic.h>
>>>  #include <linux/posix_acl_xattr.h>
>>>  
>>> @@ -566,9 +567,9 @@ static int evm_protect_xattr(struct mnt_idmap *idmap,
>>>   * userspace from writing HMAC value.  Writing 'security.evm' requires
>>>   * requires CAP_SYS_ADMIN privileges.
>>>   */
>>> -int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
>>> -		       const char *xattr_name, const void *xattr_value,
>>> -		       size_t xattr_value_len, int flags)
>>> +static int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
>>> +			      const char *xattr_name, const void *xattr_value,
>>> +			      size_t xattr_value_len, int flags)
>>>  {
>>>  	const struct evm_ima_xattr_data *xattr_data = xattr_value;
>>>  
>>> @@ -598,8 +599,8 @@ int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
>>>   * Removing 'security.evm' requires CAP_SYS_ADMIN privileges and that
>>>   * the current value is valid.
>>>   */
>>> -int evm_inode_removexattr(struct mnt_idmap *idmap,
>>> -			  struct dentry *dentry, const char *xattr_name)
>>> +static int evm_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry,
>>> +				 const char *xattr_name)
>>>  {
>>>  	/* Policy permits modification of the protected xattrs even though
>>>  	 * there's no HMAC key loaded
>>> @@ -649,9 +650,11 @@ static inline int evm_inode_set_acl_change(struct mnt_idmap *idmap,
>>>   * Prevent modifying posix acls causing the EVM HMAC to be re-calculated
>>>   * and 'security.evm' xattr updated, unless the existing 'security.evm' is
>>>   * valid.
>>> + *
>>> + * Return: zero on success, -EPERM on failure.
>>>   */
>>> -int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>>> -		      const char *acl_name, struct posix_acl *kacl)
>>> +static int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>>> +			     const char *acl_name, struct posix_acl *kacl)
>>>  {
>>>  	enum integrity_status evm_status;
>>>  
>>> @@ -690,6 +693,24 @@ int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>>>  	return -EPERM;
>>>  }
>>>  
>>> +/**
>>> + * evm_inode_remove_acl - Protect the EVM extended attribute from posix acls
>>> + * @idmap: idmap of the mount
>>> + * @dentry: pointer to the affected dentry
>>> + * @acl_name: name of the posix acl
>>> + *
>>> + * Prevent removing posix acls causing the EVM HMAC to be re-calculated
>>> + * and 'security.evm' xattr updated, unless the existing 'security.evm' is
>>> + * valid.
>>> + *
>>> + * Return: zero on success, -EPERM on failure.
>>> + */
>>> +static int evm_inode_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>>> +				const char *acl_name)
>>> +{
>>> +	return evm_inode_set_acl(idmap, dentry, acl_name, NULL);
>>> +}
>>> +
>>>  static void evm_reset_status(struct inode *inode)
>>>  {
>>>  	struct integrity_iint_cache *iint;
>>> @@ -738,9 +759,11 @@ bool evm_revalidate_status(const char *xattr_name)
>>>   * __vfs_setxattr_noperm().  The caller of which has taken the inode's
>>>   * i_mutex lock.
>>>   */
>>> -void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
>>> -			     const void *xattr_value, size_t xattr_value_len,
>>> -			     int flags)
>>> +static void evm_inode_post_setxattr(struct dentry *dentry,
>>> +				    const char *xattr_name,
>>> +				    const void *xattr_value,
>>> +				    size_t xattr_value_len,
>>> +				    int flags)
>>>  {
>>>  	if (!evm_revalidate_status(xattr_name))
>>>  		return;
>>> @@ -756,6 +779,21 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
>>>  	evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len);
>>>  }
>>>  
>>> +/**
>>> + * evm_inode_post_set_acl - Update the EVM extended attribute from posix acls
>>> + * @dentry: pointer to the affected dentry
>>> + * @acl_name: name of the posix acl
>>> + * @kacl: pointer to the posix acls
>>> + *
>>> + * Update the 'security.evm' xattr with the EVM HMAC re-calculated after setting
>>> + * posix acls.
>>> + */
>>> +static void evm_inode_post_set_acl(struct dentry *dentry, const char *acl_name,
>>> +				   struct posix_acl *kacl)
>>> +{
>>> +	return evm_inode_post_setxattr(dentry, acl_name, NULL, 0, 0);
>>> +}
>>> +
>>>  /**
>>>   * evm_inode_post_removexattr - update 'security.evm' after removing the xattr
>>>   * @dentry: pointer to the affected dentry
>>> @@ -766,7 +804,8 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
>>>   * No need to take the i_mutex lock here, as this function is called from
>>>   * vfs_removexattr() which takes the i_mutex.
>>>   */
>>> -void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
>>> +static void evm_inode_post_removexattr(struct dentry *dentry,
>>> +				       const char *xattr_name)
>>>  {
>>>  	if (!evm_revalidate_status(xattr_name))
>>>  		return;
>>> @@ -782,6 +821,22 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
>>>  	evm_update_evmxattr(dentry, xattr_name, NULL, 0);
>>>  }
>>>  
>>> +/**
>>> + * evm_inode_post_remove_acl - Update the EVM extended attribute from posix acls
>>> + * @idmap: idmap of the mount
>>> + * @dentry: pointer to the affected dentry
>>> + * @acl_name: name of the posix acl
>>> + *
>>> + * Update the 'security.evm' xattr with the EVM HMAC re-calculated after
>>> + * removing posix acls.
>>> + */
>>> +static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
>>> +					     struct dentry *dentry,
>>> +					     const char *acl_name)
>>> +{
>>> +	evm_inode_post_removexattr(dentry, acl_name);
>>> +}
>>> +
>>>  static int evm_attr_change(struct mnt_idmap *idmap,
>>>  			   struct dentry *dentry, struct iattr *attr)
>>>  {
>>> @@ -805,8 +860,8 @@ static int evm_attr_change(struct mnt_idmap *idmap,
>>>   * Permit update of file attributes when files have a valid EVM signature,
>>>   * except in the case of them having an immutable portable signature.
>>>   */
>>> -int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>>> -		      struct iattr *attr)
>>> +static int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>>> +			     struct iattr *attr)
>>>  {
>>>  	unsigned int ia_valid = attr->ia_valid;
>>>  	enum integrity_status evm_status;
>>> @@ -853,8 +908,8 @@ int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>>>   * This function is called from notify_change(), which expects the caller
>>>   * to lock the inode's i_mutex.
>>>   */
>>> -void evm_inode_post_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>>> -			    int ia_valid)
>>> +static void evm_inode_post_setattr(struct mnt_idmap *idmap,
>>> +				   struct dentry *dentry, int ia_valid)
>>>  {
>>>  	if (!evm_revalidate_status(NULL))
>>>  		return;
>>> @@ -892,7 +947,7 @@ int evm_inode_init_security(struct inode *inode, struct inode *dir,
>>>  	if (!evm_protected_xattrs)
>>>  		return -EOPNOTSUPP;
>>>  
>>> -	evm_xattr = xattr;
>>> +	evm_xattr = xattrs + integrity_blob_sizes.lbs_xattr;
>> Please don't do this inline. Convention is to use a function,
>> intergrity_xattrs() for this.
> Ok.
>
>>>  
>>>  	xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);
>>>  	if (!xattr_data)
>>> @@ -952,4 +1007,23 @@ static int __init init_evm(void)
>>>  	return error;
>>>  }
>>>  
>>> +static struct security_hook_list evm_hooks[] __lsm_ro_after_init = {
>>> +	LSM_HOOK_INIT(inode_setattr, evm_inode_setattr),
>>> +	LSM_HOOK_INIT(inode_post_setattr, evm_inode_post_setattr),
>>> +	LSM_HOOK_INIT(inode_setxattr, evm_inode_setxattr),
>>> +	LSM_HOOK_INIT(inode_set_acl, evm_inode_set_acl),
>>> +	LSM_HOOK_INIT(inode_post_set_acl, evm_inode_post_set_acl),
>>> +	LSM_HOOK_INIT(inode_remove_acl, evm_inode_remove_acl),
>>> +	LSM_HOOK_INIT(inode_post_remove_acl, evm_inode_post_remove_acl),
>>> +	LSM_HOOK_INIT(inode_post_setxattr, evm_inode_post_setxattr),
>>> +	LSM_HOOK_INIT(inode_removexattr, evm_inode_removexattr),
>>> +	LSM_HOOK_INIT(inode_post_removexattr, evm_inode_post_removexattr),
>>> +	LSM_HOOK_INIT(inode_init_security, evm_inode_init_security),
>>> +};
>>> +
>>> +void __init init_evm_lsm(void)
>>> +{
>>> +	security_add_hooks(evm_hooks, ARRAY_SIZE(evm_hooks), "integrity");
>>> +}
>>> +
>>>  late_initcall(init_evm);
>>> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
>>> index bbadf974b31..952d5ea4e18 100644
>>> --- a/security/integrity/iint.c
>>> +++ b/security/integrity/iint.c
>>> @@ -179,12 +179,19 @@ static int __init integrity_lsm_init(void)
>>>  			      0, SLAB_PANIC, init_once);
>>>  
>>>  	init_ima_lsm();
>>> +	init_evm_lsm();
>>>  	return 0;
>>>  }
>>> +
>>> +struct lsm_blob_sizes integrity_blob_sizes __lsm_ro_after_init = {
>>> +	.lbs_xattr = 1,
>> Really? 1 byte? Don't even think of storing number of elements in lbs_xattr.
>> The linux_blob_size structure contains sizes of blobs, not number of elements.
> Oh, I see it can be confusing.
>
> However, lbs_xattr does not help to position in the security blob but
> in the new_xattrs array, allocated in security_inode_init_security()
> (see below). Any suggestion on how to make this part better?

On further review, your current use is perfectly reasonable.
The patch that introduces lbs_xattr (not in this set, I see)
needs to document the use so other LSMs can use is correctly.

>
> Thanks
>
> Roberto
>
>>> +};
>>> +
>>>  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 c72d375a356..76e7eda6651 100644
>>> --- a/security/integrity/integrity.h
>>> +++ b/security/integrity/integrity.h
>>> @@ -188,6 +188,7 @@ 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;
>>>  
>>>  struct modsig;
>>>  
>>> @@ -199,6 +200,14 @@ static inline void __init init_ima_lsm(void)
>>>  }
>>>  #endif
>>>  
>>> +#ifdef CONFIG_EVM
>>> +void __init init_evm_lsm(void);
>>> +#else
>>> +static inline void __init init_evm_lsm(void)
>>> +{
>>> +}
>>> +#endif
>>> +
>>>  #ifdef CONFIG_INTEGRITY_SIGNATURE
>>>  
>>>  int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
>>> diff --git a/security/security.c b/security/security.c
>>> index 9bc6a4ef758..74abf04feef 100644
>>> --- a/security/security.c
>>> +++ b/security/security.c
>>> @@ -20,13 +20,13 @@
>>>  #include <linux/kernel_read_file.h>
>>>  #include <linux/lsm_hooks.h>
>>>  #include <linux/integrity.h>
>>> -#include <linux/evm.h>
>>>  #include <linux/fsnotify.h>
>>>  #include <linux/mman.h>
>>>  #include <linux/mount.h>
>>>  #include <linux/personality.h>
>>>  #include <linux/backing-dev.h>
>>>  #include <linux/string.h>
>>> +#include <linux/xattr.h>
>>>  #include <linux/msg.h>
>>>  #include <net/flow.h>
>>>  
>>> @@ -1662,8 +1662,8 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
>>>  	if (!initxattrs)
>>>  		return call_int_hook(inode_init_security, -EOPNOTSUPP, inode,
>>>  				    dir, qstr, NULL);
>>> -	/* Allocate +1 for EVM and +1 as terminator. */
>>> -	new_xattrs = kcalloc(blob_sizes.lbs_xattr + 2, sizeof(*new_xattrs),
>>> +	/* Allocate +1 for terminator. */
>>> +	new_xattrs = kcalloc(blob_sizes.lbs_xattr + 1, sizeof(*new_xattrs),
>>>  			     GFP_NOFS);
>>>  	if (!new_xattrs)
>>>  		return -ENOMEM;
>>> @@ -1699,9 +1699,6 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
>>>  	if (!num_filled_xattrs)
>>>  		goto out;
>>>  
>>> -	ret = evm_inode_init_security(inode, dir, qstr, new_xattrs);
>>> -	if (ret && ret != -EOPNOTSUPP)
>>> -		goto out;
>>>  	ret = initxattrs(inode, new_xattrs, fs_data);
>>>  out:
>>>  	for (xattr = new_xattrs; xattr->value != NULL; xattr++)
>>> @@ -2201,14 +2198,9 @@ int security_inode_permission(struct inode *inode, int mask)
>>>  int security_inode_setattr(struct mnt_idmap *idmap,
>>>  			   struct dentry *dentry, struct iattr *attr)
>>>  {
>>> -	int ret;
>>> -
>>>  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>>>  		return 0;
>>> -	ret = call_int_hook(inode_setattr, 0, idmap, dentry, attr);
>>> -	if (ret)
>>> -		return ret;
>>> -	return evm_inode_setattr(idmap, dentry, attr);
>>> +	return call_int_hook(inode_setattr, 0, idmap, dentry, attr);
>>>  }
>>>  EXPORT_SYMBOL_GPL(security_inode_setattr);
>>>  
>>> @@ -2272,9 +2264,7 @@ int security_inode_setxattr(struct mnt_idmap *idmap,
>>>  
>>>  	if (ret == 1)
>>>  		ret = cap_inode_setxattr(dentry, name, value, size, flags);
>>> -	if (ret)
>>> -		return ret;
>>> -	return evm_inode_setxattr(idmap, dentry, name, value, size, flags);
>>> +	return ret;
>>>  }
>>>  
>>>  /**
>>> @@ -2293,15 +2283,10 @@ int security_inode_set_acl(struct mnt_idmap *idmap,
>>>  			   struct dentry *dentry, const char *acl_name,
>>>  			   struct posix_acl *kacl)
>>>  {
>>> -	int ret;
>>> -
>>>  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>>>  		return 0;
>>> -	ret = call_int_hook(inode_set_acl, 0, idmap, dentry, acl_name,
>>> -			    kacl);
>>> -	if (ret)
>>> -		return ret;
>>> -	return evm_inode_set_acl(idmap, dentry, acl_name, kacl);
>>> +	return call_int_hook(inode_set_acl, 0, idmap, dentry, acl_name,
>>> +			     kacl);
>>>  }
>>>  
>>>  /**
>>> @@ -2354,14 +2339,9 @@ int security_inode_get_acl(struct mnt_idmap *idmap,
>>>  int security_inode_remove_acl(struct mnt_idmap *idmap,
>>>  			      struct dentry *dentry, const char *acl_name)
>>>  {
>>> -	int ret;
>>> -
>>>  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>>>  		return 0;
>>> -	ret = call_int_hook(inode_remove_acl, 0, idmap, dentry, acl_name);
>>> -	if (ret)
>>> -		return ret;
>>> -	return evm_inode_remove_acl(idmap, dentry, acl_name);
>>> +	return call_int_hook(inode_remove_acl, 0, idmap, dentry, acl_name);
>>>  }
>>>  
>>>  /**
>>> @@ -2397,7 +2377,6 @@ void security_inode_post_setxattr(struct dentry *dentry, const char *name,
>>>  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
>>>  		return;
>>>  	call_void_hook(inode_post_setxattr, dentry, name, value, size, flags);
>>> -	evm_inode_post_setxattr(dentry, name, value, size, flags);
>>>  }
>>>  
>>>  /**
>>> @@ -2458,9 +2437,7 @@ int security_inode_removexattr(struct mnt_idmap *idmap,
>>>  	ret = call_int_hook(inode_removexattr, 1, idmap, dentry, name);
>>>  	if (ret == 1)
>>>  		ret = cap_inode_removexattr(idmap, dentry, name);
>>> -	if (ret)
>>> -		return ret;
>>> -	return evm_inode_removexattr(idmap, dentry, name);
>>> +	return ret;
>>>  }
>>>  
>>>  /**

Roberto Sassu March 7, 2023, 4:57 p.m. UTC | #4

On Tue, 2023-03-07 at 08:54 -0800, Casey Schaufler wrote:
> On 3/6/2023 1:21 AM, Roberto Sassu wrote:
> > On Sat, 2023-03-04 at 13:36 -0800, Casey Schaufler wrote:
> > > On 3/3/2023 10:26 AM, Roberto Sassu wrote:
> > > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > > > 
> > > > As for IMA, remove hardcoded EVM function calls from the LSM infrastructure
> > > > and the VFS. Make EVM functions as static (except for
> > > > evm_inode_init_security(), which is exported), and register them as hook
> > > > implementations in init_evm_lsm(), called from integrity_lsm_init().
> > > > 
> > > > Finally, switch to the LSM reservation mechanism for the EVM xattr, by
> > > > setting the lbs_xattr field of the lsm_blob_sizes structure, and
> > > > consequently decrement the number of xattrs to allocate in
> > > > security_inode_init_security().
> > > > 
> > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > > > ---
> > > >  fs/attr.c                         |   2 -
> > > >  fs/posix_acl.c                    |   3 -
> > > >  fs/xattr.c                        |   2 -
> > > >  include/linux/evm.h               | 116 ------------------------------
> > > >  security/integrity/evm/evm_main.c | 106 ++++++++++++++++++++++-----
> > > >  security/integrity/iint.c         |   7 ++
> > > >  security/integrity/integrity.h    |   9 +++
> > > >  security/security.c               |  41 +++--------
> > > >  8 files changed, 115 insertions(+), 171 deletions(-)
> > > > 
> > > > diff --git a/fs/attr.c b/fs/attr.c
> > > > index 406d782dfab..1b911a627fe 100644
> > > > --- a/fs/attr.c
> > > > +++ b/fs/attr.c
> > > > @@ -16,7 +16,6 @@
> > > >  #include <linux/fcntl.h>
> > > >  #include <linux/filelock.h>
> > > >  #include <linux/security.h>
> > > > -#include <linux/evm.h>
> > > >  
> > > >  #include "internal.h"
> > > >  
> > > > @@ -485,7 +484,6 @@ int notify_change(struct mnt_idmap *idmap, struct dentry *dentry,
> > > >  	if (!error) {
> > > >  		fsnotify_change(dentry, ia_valid);
> > > >  		security_inode_post_setattr(idmap, dentry, ia_valid);
> > > > -		evm_inode_post_setattr(idmap, dentry, ia_valid);
> > > >  	}
> > > >  
> > > >  	return error;
> > > > diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> > > > index 5b8c92fce0c..608cb0a9f84 100644
> > > > --- a/fs/posix_acl.c
> > > > +++ b/fs/posix_acl.c
> > > > @@ -26,7 +26,6 @@
> > > >  #include <linux/mnt_idmapping.h>
> > > >  #include <linux/iversion.h>
> > > >  #include <linux/security.h>
> > > > -#include <linux/evm.h>
> > > >  #include <linux/fsnotify.h>
> > > >  #include <linux/filelock.h>
> > > >  
> > > > @@ -1103,7 +1102,6 @@ int vfs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> > > >  	if (!error) {
> > > >  		fsnotify_xattr(dentry);
> > > >  		security_inode_post_set_acl(dentry, acl_name, kacl);
> > > > -		evm_inode_post_set_acl(dentry, acl_name, kacl);
> > > >  	}
> > > >  
> > > >  out_inode_unlock:
> > > > @@ -1214,7 +1212,6 @@ int vfs_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> > > >  	if (!error) {
> > > >  		fsnotify_xattr(dentry);
> > > >  		security_inode_post_remove_acl(idmap, dentry, acl_name);
> > > > -		evm_inode_post_remove_acl(idmap, dentry, acl_name);
> > > >  	}
> > > >  
> > > >  out_inode_unlock:
> > > > diff --git a/fs/xattr.c b/fs/xattr.c
> > > > index 10c959d9fc6..7708ffdacca 100644
> > > > --- a/fs/xattr.c
> > > > +++ b/fs/xattr.c
> > > > @@ -16,7 +16,6 @@
> > > >  #include <linux/mount.h>
> > > >  #include <linux/namei.h>
> > > >  #include <linux/security.h>
> > > > -#include <linux/evm.h>
> > > >  #include <linux/syscalls.h>
> > > >  #include <linux/export.h>
> > > >  #include <linux/fsnotify.h>
> > > > @@ -535,7 +534,6 @@ __vfs_removexattr_locked(struct mnt_idmap *idmap,
> > > >  	if (!error) {
> > > >  		fsnotify_xattr(dentry);
> > > >  		security_inode_post_removexattr(dentry, name);
> > > > -		evm_inode_post_removexattr(dentry, name);
> > > >  	}
> > > >  
> > > >  out:
> > > > diff --git a/include/linux/evm.h b/include/linux/evm.h
> > > > index 8c043273552..61794299f09 100644
> > > > --- a/include/linux/evm.h
> > > > +++ b/include/linux/evm.h
> > > > @@ -21,46 +21,6 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
> > > >  					     void *xattr_value,
> > > >  					     size_t xattr_value_len,
> > > >  					     struct integrity_iint_cache *iint);
> > > > -extern int evm_inode_setattr(struct mnt_idmap *idmap,
> > > > -			     struct dentry *dentry, struct iattr *attr);
> > > > -extern void evm_inode_post_setattr(struct mnt_idmap *idmap,
> > > > -				   struct dentry *dentry, int ia_valid);
> > > > -extern int evm_inode_setxattr(struct mnt_idmap *idmap,
> > > > -			      struct dentry *dentry, const char *name,
> > > > -			      const void *value, size_t size, int flags);
> > > > -extern void evm_inode_post_setxattr(struct dentry *dentry,
> > > > -				    const char *xattr_name,
> > > > -				    const void *xattr_value,
> > > > -				    size_t xattr_value_len,
> > > > -				    int flags);
> > > > -extern int evm_inode_removexattr(struct mnt_idmap *idmap,
> > > > -				 struct dentry *dentry, const char *xattr_name);
> > > > -extern void evm_inode_post_removexattr(struct dentry *dentry,
> > > > -				       const char *xattr_name);
> > > > -static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
> > > > -					     struct dentry *dentry,
> > > > -					     const char *acl_name)
> > > > -{
> > > > -	evm_inode_post_removexattr(dentry, acl_name);
> > > > -}
> > > > -extern int evm_inode_set_acl(struct mnt_idmap *idmap,
> > > > -			     struct dentry *dentry, const char *acl_name,
> > > > -			     struct posix_acl *kacl);
> > > > -static inline int evm_inode_remove_acl(struct mnt_idmap *idmap,
> > > > -				       struct dentry *dentry,
> > > > -				       const char *acl_name)
> > > > -{
> > > > -	return evm_inode_set_acl(idmap, dentry, acl_name, NULL);
> > > > -}
> > > > -static inline void evm_inode_post_set_acl(struct dentry *dentry,
> > > > -					  const char *acl_name,
> > > > -					  struct posix_acl *kacl)
> > > > -{
> > > > -	return evm_inode_post_setxattr(dentry, acl_name, NULL, 0, 0);
> > > > -}
> > > > -extern int evm_inode_init_security(struct inode *inode, struct inode *dir,
> > > > -				   const struct qstr *qstr,
> > > > -				   struct xattr *xattrs);
> > > >  extern bool evm_revalidate_status(const char *xattr_name);
> > > >  extern int evm_protected_xattr_if_enabled(const char *req_xattr_name);
> > > >  extern int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
> > > > @@ -92,82 +52,6 @@ static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
> > > >  }
> > > >  #endif
> > > >  
> > > > -static inline int evm_inode_setattr(struct mnt_idmap *idmap,
> > > > -				    struct dentry *dentry, struct iattr *attr)
> > > > -{
> > > > -	return 0;
> > > > -}
> > > > -
> > > > -static inline void evm_inode_post_setattr(struct mnt_idmap *idmap,
> > > > -					  struct dentry *dentry, int ia_valid)
> > > > -{
> > > > -	return;
> > > > -}
> > > > -
> > > > -static inline int evm_inode_setxattr(struct mnt_idmap *idmap,
> > > > -				     struct dentry *dentry, const char *name,
> > > > -				     const void *value, size_t size, int flags)
> > > > -{
> > > > -	return 0;
> > > > -}
> > > > -
> > > > -static inline void evm_inode_post_setxattr(struct dentry *dentry,
> > > > -					   const char *xattr_name,
> > > > -					   const void *xattr_value,
> > > > -					   size_t xattr_value_len,
> > > > -					   int flags)
> > > > -{
> > > > -	return;
> > > > -}
> > > > -
> > > > -static inline int evm_inode_removexattr(struct mnt_idmap *idmap,
> > > > -					struct dentry *dentry,
> > > > -					const char *xattr_name)
> > > > -{
> > > > -	return 0;
> > > > -}
> > > > -
> > > > -static inline void evm_inode_post_removexattr(struct dentry *dentry,
> > > > -					      const char *xattr_name)
> > > > -{
> > > > -	return;
> > > > -}
> > > > -
> > > > -static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
> > > > -					     struct dentry *dentry,
> > > > -					     const char *acl_name)
> > > > -{
> > > > -	return;
> > > > -}
> > > > -
> > > > -static inline int evm_inode_set_acl(struct mnt_idmap *idmap,
> > > > -				    struct dentry *dentry, const char *acl_name,
> > > > -				    struct posix_acl *kacl)
> > > > -{
> > > > -	return 0;
> > > > -}
> > > > -
> > > > -static inline int evm_inode_remove_acl(struct mnt_idmap *idmap,
> > > > -				       struct dentry *dentry,
> > > > -				       const char *acl_name)
> > > > -{
> > > > -	return 0;
> > > > -}
> > > > -
> > > > -static inline void evm_inode_post_set_acl(struct dentry *dentry,
> > > > -					  const char *acl_name,
> > > > -					  struct posix_acl *kacl)
> > > > -{
> > > > -	return;
> > > > -}
> > > > -
> > > > -static inline int evm_inode_init_security(struct inode *inode, struct inode *dir,
> > > > -					  const struct qstr *qstr,
> > > > -					  struct xattr *xattrs)
> > > > -{
> > > > -	return 0;
> > > > -}
> > > > -
> > > >  static inline bool evm_revalidate_status(const char *xattr_name)
> > > >  {
> > > >  	return false;
> > > > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > > > index 8b5c472f78b..c45bc97277c 100644
> > > > --- a/security/integrity/evm/evm_main.c
> > > > +++ b/security/integrity/evm/evm_main.c
> > > > @@ -19,6 +19,7 @@
> > > >  #include <linux/xattr.h>
> > > >  #include <linux/integrity.h>
> > > >  #include <linux/evm.h>
> > > > +#include <linux/lsm_hooks.h>
> > > >  #include <linux/magic.h>
> > > >  #include <linux/posix_acl_xattr.h>
> > > >  
> > > > @@ -566,9 +567,9 @@ static int evm_protect_xattr(struct mnt_idmap *idmap,
> > > >   * userspace from writing HMAC value.  Writing 'security.evm' requires
> > > >   * requires CAP_SYS_ADMIN privileges.
> > > >   */
> > > > -int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > > > -		       const char *xattr_name, const void *xattr_value,
> > > > -		       size_t xattr_value_len, int flags)
> > > > +static int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > > > +			      const char *xattr_name, const void *xattr_value,
> > > > +			      size_t xattr_value_len, int flags)
> > > >  {
> > > >  	const struct evm_ima_xattr_data *xattr_data = xattr_value;
> > > >  
> > > > @@ -598,8 +599,8 @@ int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > > >   * Removing 'security.evm' requires CAP_SYS_ADMIN privileges and that
> > > >   * the current value is valid.
> > > >   */
> > > > -int evm_inode_removexattr(struct mnt_idmap *idmap,
> > > > -			  struct dentry *dentry, const char *xattr_name)
> > > > +static int evm_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > > > +				 const char *xattr_name)
> > > >  {
> > > >  	/* Policy permits modification of the protected xattrs even though
> > > >  	 * there's no HMAC key loaded
> > > > @@ -649,9 +650,11 @@ static inline int evm_inode_set_acl_change(struct mnt_idmap *idmap,
> > > >   * Prevent modifying posix acls causing the EVM HMAC to be re-calculated
> > > >   * and 'security.evm' xattr updated, unless the existing 'security.evm' is
> > > >   * valid.
> > > > + *
> > > > + * Return: zero on success, -EPERM on failure.
> > > >   */
> > > > -int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> > > > -		      const char *acl_name, struct posix_acl *kacl)
> > > > +static int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> > > > +			     const char *acl_name, struct posix_acl *kacl)
> > > >  {
> > > >  	enum integrity_status evm_status;
> > > >  
> > > > @@ -690,6 +693,24 @@ int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> > > >  	return -EPERM;
> > > >  }
> > > >  
> > > > +/**
> > > > + * evm_inode_remove_acl - Protect the EVM extended attribute from posix acls
> > > > + * @idmap: idmap of the mount
> > > > + * @dentry: pointer to the affected dentry
> > > > + * @acl_name: name of the posix acl
> > > > + *
> > > > + * Prevent removing posix acls causing the EVM HMAC to be re-calculated
> > > > + * and 'security.evm' xattr updated, unless the existing 'security.evm' is
> > > > + * valid.
> > > > + *
> > > > + * Return: zero on success, -EPERM on failure.
> > > > + */
> > > > +static int evm_inode_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> > > > +				const char *acl_name)
> > > > +{
> > > > +	return evm_inode_set_acl(idmap, dentry, acl_name, NULL);
> > > > +}
> > > > +
> > > >  static void evm_reset_status(struct inode *inode)
> > > >  {
> > > >  	struct integrity_iint_cache *iint;
> > > > @@ -738,9 +759,11 @@ bool evm_revalidate_status(const char *xattr_name)
> > > >   * __vfs_setxattr_noperm().  The caller of which has taken the inode's
> > > >   * i_mutex lock.
> > > >   */
> > > > -void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
> > > > -			     const void *xattr_value, size_t xattr_value_len,
> > > > -			     int flags)
> > > > +static void evm_inode_post_setxattr(struct dentry *dentry,
> > > > +				    const char *xattr_name,
> > > > +				    const void *xattr_value,
> > > > +				    size_t xattr_value_len,
> > > > +				    int flags)
> > > >  {
> > > >  	if (!evm_revalidate_status(xattr_name))
> > > >  		return;
> > > > @@ -756,6 +779,21 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
> > > >  	evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len);
> > > >  }
> > > >  
> > > > +/**
> > > > + * evm_inode_post_set_acl - Update the EVM extended attribute from posix acls
> > > > + * @dentry: pointer to the affected dentry
> > > > + * @acl_name: name of the posix acl
> > > > + * @kacl: pointer to the posix acls
> > > > + *
> > > > + * Update the 'security.evm' xattr with the EVM HMAC re-calculated after setting
> > > > + * posix acls.
> > > > + */
> > > > +static void evm_inode_post_set_acl(struct dentry *dentry, const char *acl_name,
> > > > +				   struct posix_acl *kacl)
> > > > +{
> > > > +	return evm_inode_post_setxattr(dentry, acl_name, NULL, 0, 0);
> > > > +}
> > > > +
> > > >  /**
> > > >   * evm_inode_post_removexattr - update 'security.evm' after removing the xattr
> > > >   * @dentry: pointer to the affected dentry
> > > > @@ -766,7 +804,8 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
> > > >   * No need to take the i_mutex lock here, as this function is called from
> > > >   * vfs_removexattr() which takes the i_mutex.
> > > >   */
> > > > -void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
> > > > +static void evm_inode_post_removexattr(struct dentry *dentry,
> > > > +				       const char *xattr_name)
> > > >  {
> > > >  	if (!evm_revalidate_status(xattr_name))
> > > >  		return;
> > > > @@ -782,6 +821,22 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
> > > >  	evm_update_evmxattr(dentry, xattr_name, NULL, 0);
> > > >  }
> > > >  
> > > > +/**
> > > > + * evm_inode_post_remove_acl - Update the EVM extended attribute from posix acls
> > > > + * @idmap: idmap of the mount
> > > > + * @dentry: pointer to the affected dentry
> > > > + * @acl_name: name of the posix acl
> > > > + *
> > > > + * Update the 'security.evm' xattr with the EVM HMAC re-calculated after
> > > > + * removing posix acls.
> > > > + */
> > > > +static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
> > > > +					     struct dentry *dentry,
> > > > +					     const char *acl_name)
> > > > +{
> > > > +	evm_inode_post_removexattr(dentry, acl_name);
> > > > +}
> > > > +
> > > >  static int evm_attr_change(struct mnt_idmap *idmap,
> > > >  			   struct dentry *dentry, struct iattr *attr)
> > > >  {
> > > > @@ -805,8 +860,8 @@ static int evm_attr_change(struct mnt_idmap *idmap,
> > > >   * Permit update of file attributes when files have a valid EVM signature,
> > > >   * except in the case of them having an immutable portable signature.
> > > >   */
> > > > -int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > > > -		      struct iattr *attr)
> > > > +static int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > > > +			     struct iattr *attr)
> > > >  {
> > > >  	unsigned int ia_valid = attr->ia_valid;
> > > >  	enum integrity_status evm_status;
> > > > @@ -853,8 +908,8 @@ int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > > >   * This function is called from notify_change(), which expects the caller
> > > >   * to lock the inode's i_mutex.
> > > >   */
> > > > -void evm_inode_post_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> > > > -			    int ia_valid)
> > > > +static void evm_inode_post_setattr(struct mnt_idmap *idmap,
> > > > +				   struct dentry *dentry, int ia_valid)
> > > >  {
> > > >  	if (!evm_revalidate_status(NULL))
> > > >  		return;
> > > > @@ -892,7 +947,7 @@ int evm_inode_init_security(struct inode *inode, struct inode *dir,
> > > >  	if (!evm_protected_xattrs)
> > > >  		return -EOPNOTSUPP;
> > > >  
> > > > -	evm_xattr = xattr;
> > > > +	evm_xattr = xattrs + integrity_blob_sizes.lbs_xattr;
> > > Please don't do this inline. Convention is to use a function,
> > > intergrity_xattrs() for this.
> > Ok.
> > 
> > > >  
> > > >  	xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);
> > > >  	if (!xattr_data)
> > > > @@ -952,4 +1007,23 @@ static int __init init_evm(void)
> > > >  	return error;
> > > >  }
> > > >  
> > > > +static struct security_hook_list evm_hooks[] __lsm_ro_after_init = {
> > > > +	LSM_HOOK_INIT(inode_setattr, evm_inode_setattr),
> > > > +	LSM_HOOK_INIT(inode_post_setattr, evm_inode_post_setattr),
> > > > +	LSM_HOOK_INIT(inode_setxattr, evm_inode_setxattr),
> > > > +	LSM_HOOK_INIT(inode_set_acl, evm_inode_set_acl),
> > > > +	LSM_HOOK_INIT(inode_post_set_acl, evm_inode_post_set_acl),
> > > > +	LSM_HOOK_INIT(inode_remove_acl, evm_inode_remove_acl),
> > > > +	LSM_HOOK_INIT(inode_post_remove_acl, evm_inode_post_remove_acl),
> > > > +	LSM_HOOK_INIT(inode_post_setxattr, evm_inode_post_setxattr),
> > > > +	LSM_HOOK_INIT(inode_removexattr, evm_inode_removexattr),
> > > > +	LSM_HOOK_INIT(inode_post_removexattr, evm_inode_post_removexattr),
> > > > +	LSM_HOOK_INIT(inode_init_security, evm_inode_init_security),
> > > > +};
> > > > +
> > > > +void __init init_evm_lsm(void)
> > > > +{
> > > > +	security_add_hooks(evm_hooks, ARRAY_SIZE(evm_hooks), "integrity");
> > > > +}
> > > > +
> > > >  late_initcall(init_evm);
> > > > diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> > > > index bbadf974b31..952d5ea4e18 100644
> > > > --- a/security/integrity/iint.c
> > > > +++ b/security/integrity/iint.c
> > > > @@ -179,12 +179,19 @@ static int __init integrity_lsm_init(void)
> > > >  			      0, SLAB_PANIC, init_once);
> > > >  
> > > >  	init_ima_lsm();
> > > > +	init_evm_lsm();
> > > >  	return 0;
> > > >  }
> > > > +
> > > > +struct lsm_blob_sizes integrity_blob_sizes __lsm_ro_after_init = {
> > > > +	.lbs_xattr = 1,
> > > Really? 1 byte? Don't even think of storing number of elements in lbs_xattr.
> > > The linux_blob_size structure contains sizes of blobs, not number of elements.
> > Oh, I see it can be confusing.
> > 
> > However, lbs_xattr does not help to position in the security blob but
> > in the new_xattrs array, allocated in security_inode_init_security()
> > (see below). Any suggestion on how to make this part better?
> 
> On further review, your current use is perfectly reasonable.
> The patch that introduces lbs_xattr (not in this set, I see)
> needs to document the use so other LSMs can use is correctly.

Ok, will add it.

So far, Paul's review should be the last.

Thanks

Roberto

> > Thanks
> > 
> > Roberto
> > 
> > > > +};
> > > > +
> > > >  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 c72d375a356..76e7eda6651 100644
> > > > --- a/security/integrity/integrity.h
> > > > +++ b/security/integrity/integrity.h
> > > > @@ -188,6 +188,7 @@ 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;
> > > >  
> > > >  struct modsig;
> > > >  
> > > > @@ -199,6 +200,14 @@ static inline void __init init_ima_lsm(void)
> > > >  }
> > > >  #endif
> > > >  
> > > > +#ifdef CONFIG_EVM
> > > > +void __init init_evm_lsm(void);
> > > > +#else
> > > > +static inline void __init init_evm_lsm(void)
> > > > +{
> > > > +}
> > > > +#endif
> > > > +
> > > >  #ifdef CONFIG_INTEGRITY_SIGNATURE
> > > >  
> > > >  int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
> > > > diff --git a/security/security.c b/security/security.c
> > > > index 9bc6a4ef758..74abf04feef 100644
> > > > --- a/security/security.c
> > > > +++ b/security/security.c
> > > > @@ -20,13 +20,13 @@
> > > >  #include <linux/kernel_read_file.h>
> > > >  #include <linux/lsm_hooks.h>
> > > >  #include <linux/integrity.h>
> > > > -#include <linux/evm.h>
> > > >  #include <linux/fsnotify.h>
> > > >  #include <linux/mman.h>
> > > >  #include <linux/mount.h>
> > > >  #include <linux/personality.h>
> > > >  #include <linux/backing-dev.h>
> > > >  #include <linux/string.h>
> > > > +#include <linux/xattr.h>
> > > >  #include <linux/msg.h>
> > > >  #include <net/flow.h>
> > > >  
> > > > @@ -1662,8 +1662,8 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> > > >  	if (!initxattrs)
> > > >  		return call_int_hook(inode_init_security, -EOPNOTSUPP, inode,
> > > >  				    dir, qstr, NULL);
> > > > -	/* Allocate +1 for EVM and +1 as terminator. */
> > > > -	new_xattrs = kcalloc(blob_sizes.lbs_xattr + 2, sizeof(*new_xattrs),
> > > > +	/* Allocate +1 for terminator. */
> > > > +	new_xattrs = kcalloc(blob_sizes.lbs_xattr + 1, sizeof(*new_xattrs),
> > > >  			     GFP_NOFS);
> > > >  	if (!new_xattrs)
> > > >  		return -ENOMEM;
> > > > @@ -1699,9 +1699,6 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> > > >  	if (!num_filled_xattrs)
> > > >  		goto out;
> > > >  
> > > > -	ret = evm_inode_init_security(inode, dir, qstr, new_xattrs);
> > > > -	if (ret && ret != -EOPNOTSUPP)
> > > > -		goto out;
> > > >  	ret = initxattrs(inode, new_xattrs, fs_data);
> > > >  out:
> > > >  	for (xattr = new_xattrs; xattr->value != NULL; xattr++)
> > > > @@ -2201,14 +2198,9 @@ int security_inode_permission(struct inode *inode, int mask)
> > > >  int security_inode_setattr(struct mnt_idmap *idmap,
> > > >  			   struct dentry *dentry, struct iattr *attr)
> > > >  {
> > > > -	int ret;
> > > > -
> > > >  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> > > >  		return 0;
> > > > -	ret = call_int_hook(inode_setattr, 0, idmap, dentry, attr);
> > > > -	if (ret)
> > > > -		return ret;
> > > > -	return evm_inode_setattr(idmap, dentry, attr);
> > > > +	return call_int_hook(inode_setattr, 0, idmap, dentry, attr);
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(security_inode_setattr);
> > > >  
> > > > @@ -2272,9 +2264,7 @@ int security_inode_setxattr(struct mnt_idmap *idmap,
> > > >  
> > > >  	if (ret == 1)
> > > >  		ret = cap_inode_setxattr(dentry, name, value, size, flags);
> > > > -	if (ret)
> > > > -		return ret;
> > > > -	return evm_inode_setxattr(idmap, dentry, name, value, size, flags);
> > > > +	return ret;
> > > >  }
> > > >  
> > > >  /**
> > > > @@ -2293,15 +2283,10 @@ int security_inode_set_acl(struct mnt_idmap *idmap,
> > > >  			   struct dentry *dentry, const char *acl_name,
> > > >  			   struct posix_acl *kacl)
> > > >  {
> > > > -	int ret;
> > > > -
> > > >  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> > > >  		return 0;
> > > > -	ret = call_int_hook(inode_set_acl, 0, idmap, dentry, acl_name,
> > > > -			    kacl);
> > > > -	if (ret)
> > > > -		return ret;
> > > > -	return evm_inode_set_acl(idmap, dentry, acl_name, kacl);
> > > > +	return call_int_hook(inode_set_acl, 0, idmap, dentry, acl_name,
> > > > +			     kacl);
> > > >  }
> > > >  
> > > >  /**
> > > > @@ -2354,14 +2339,9 @@ int security_inode_get_acl(struct mnt_idmap *idmap,
> > > >  int security_inode_remove_acl(struct mnt_idmap *idmap,
> > > >  			      struct dentry *dentry, const char *acl_name)
> > > >  {
> > > > -	int ret;
> > > > -
> > > >  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> > > >  		return 0;
> > > > -	ret = call_int_hook(inode_remove_acl, 0, idmap, dentry, acl_name);
> > > > -	if (ret)
> > > > -		return ret;
> > > > -	return evm_inode_remove_acl(idmap, dentry, acl_name);
> > > > +	return call_int_hook(inode_remove_acl, 0, idmap, dentry, acl_name);
> > > >  }
> > > >  
> > > >  /**
> > > > @@ -2397,7 +2377,6 @@ void security_inode_post_setxattr(struct dentry *dentry, const char *name,
> > > >  	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
> > > >  		return;
> > > >  	call_void_hook(inode_post_setxattr, dentry, name, value, size, flags);
> > > > -	evm_inode_post_setxattr(dentry, name, value, size, flags);
> > > >  }
> > > >  
> > > >  /**
> > > > @@ -2458,9 +2437,7 @@ int security_inode_removexattr(struct mnt_idmap *idmap,
> > > >  	ret = call_int_hook(inode_removexattr, 1, idmap, dentry, name);
> > > >  	if (ret == 1)
> > > >  		ret = cap_inode_removexattr(idmap, dentry, name);
> > > > -	if (ret)
> > > > -		return ret;
> > > > -	return evm_inode_removexattr(idmap, dentry, name);
> > > > +	return ret;
> > > >  }
> > > >  
> > > >  /**

diff mbox series

Patch

diff --git a/fs/attr.c b/fs/attr.c
index 406d782dfab..1b911a627fe 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -16,7 +16,6 @@ 
 #include <linux/fcntl.h>
 #include <linux/filelock.h>
 #include <linux/security.h>
-#include <linux/evm.h>
 
 #include "internal.h"
 
@@ -485,7 +484,6 @@  int notify_change(struct mnt_idmap *idmap, struct dentry *dentry,
 	if (!error) {
 		fsnotify_change(dentry, ia_valid);
 		security_inode_post_setattr(idmap, dentry, ia_valid);
-		evm_inode_post_setattr(idmap, dentry, ia_valid);
 	}
 
 	return error;
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 5b8c92fce0c..608cb0a9f84 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -26,7 +26,6 @@ 
 #include <linux/mnt_idmapping.h>
 #include <linux/iversion.h>
 #include <linux/security.h>
-#include <linux/evm.h>
 #include <linux/fsnotify.h>
 #include <linux/filelock.h>
 
@@ -1103,7 +1102,6 @@  int vfs_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
 	if (!error) {
 		fsnotify_xattr(dentry);
 		security_inode_post_set_acl(dentry, acl_name, kacl);
-		evm_inode_post_set_acl(dentry, acl_name, kacl);
 	}
 
 out_inode_unlock:
@@ -1214,7 +1212,6 @@  int vfs_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry,
 	if (!error) {
 		fsnotify_xattr(dentry);
 		security_inode_post_remove_acl(idmap, dentry, acl_name);
-		evm_inode_post_remove_acl(idmap, dentry, acl_name);
 	}
 
 out_inode_unlock:
diff --git a/fs/xattr.c b/fs/xattr.c
index 10c959d9fc6..7708ffdacca 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -16,7 +16,6 @@ 
 #include <linux/mount.h>
 #include <linux/namei.h>
 #include <linux/security.h>
-#include <linux/evm.h>
 #include <linux/syscalls.h>
 #include <linux/export.h>
 #include <linux/fsnotify.h>
@@ -535,7 +534,6 @@  __vfs_removexattr_locked(struct mnt_idmap *idmap,
 	if (!error) {
 		fsnotify_xattr(dentry);
 		security_inode_post_removexattr(dentry, name);
-		evm_inode_post_removexattr(dentry, name);
 	}
 
 out:
diff --git a/include/linux/evm.h b/include/linux/evm.h
index 8c043273552..61794299f09 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -21,46 +21,6 @@  extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
 					     void *xattr_value,
 					     size_t xattr_value_len,
 					     struct integrity_iint_cache *iint);
-extern int evm_inode_setattr(struct mnt_idmap *idmap,
-			     struct dentry *dentry, struct iattr *attr);
-extern void evm_inode_post_setattr(struct mnt_idmap *idmap,
-				   struct dentry *dentry, int ia_valid);
-extern int evm_inode_setxattr(struct mnt_idmap *idmap,
-			      struct dentry *dentry, const char *name,
-			      const void *value, size_t size, int flags);
-extern void evm_inode_post_setxattr(struct dentry *dentry,
-				    const char *xattr_name,
-				    const void *xattr_value,
-				    size_t xattr_value_len,
-				    int flags);
-extern int evm_inode_removexattr(struct mnt_idmap *idmap,
-				 struct dentry *dentry, const char *xattr_name);
-extern void evm_inode_post_removexattr(struct dentry *dentry,
-				       const char *xattr_name);
-static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
-					     struct dentry *dentry,
-					     const char *acl_name)
-{
-	evm_inode_post_removexattr(dentry, acl_name);
-}
-extern int evm_inode_set_acl(struct mnt_idmap *idmap,
-			     struct dentry *dentry, const char *acl_name,
-			     struct posix_acl *kacl);
-static inline int evm_inode_remove_acl(struct mnt_idmap *idmap,
-				       struct dentry *dentry,
-				       const char *acl_name)
-{
-	return evm_inode_set_acl(idmap, dentry, acl_name, NULL);
-}
-static inline void evm_inode_post_set_acl(struct dentry *dentry,
-					  const char *acl_name,
-					  struct posix_acl *kacl)
-{
-	return evm_inode_post_setxattr(dentry, acl_name, NULL, 0, 0);
-}
-extern int evm_inode_init_security(struct inode *inode, struct inode *dir,
-				   const struct qstr *qstr,
-				   struct xattr *xattrs);
 extern bool evm_revalidate_status(const char *xattr_name);
 extern int evm_protected_xattr_if_enabled(const char *req_xattr_name);
 extern int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
@@ -92,82 +52,6 @@  static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
 }
 #endif
 
-static inline int evm_inode_setattr(struct mnt_idmap *idmap,
-				    struct dentry *dentry, struct iattr *attr)
-{
-	return 0;
-}
-
-static inline void evm_inode_post_setattr(struct mnt_idmap *idmap,
-					  struct dentry *dentry, int ia_valid)
-{
-	return;
-}
-
-static inline int evm_inode_setxattr(struct mnt_idmap *idmap,
-				     struct dentry *dentry, const char *name,
-				     const void *value, size_t size, int flags)
-{
-	return 0;
-}
-
-static inline void evm_inode_post_setxattr(struct dentry *dentry,
-					   const char *xattr_name,
-					   const void *xattr_value,
-					   size_t xattr_value_len,
-					   int flags)
-{
-	return;
-}
-
-static inline int evm_inode_removexattr(struct mnt_idmap *idmap,
-					struct dentry *dentry,
-					const char *xattr_name)
-{
-	return 0;
-}
-
-static inline void evm_inode_post_removexattr(struct dentry *dentry,
-					      const char *xattr_name)
-{
-	return;
-}
-
-static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
-					     struct dentry *dentry,
-					     const char *acl_name)
-{
-	return;
-}
-
-static inline int evm_inode_set_acl(struct mnt_idmap *idmap,
-				    struct dentry *dentry, const char *acl_name,
-				    struct posix_acl *kacl)
-{
-	return 0;
-}
-
-static inline int evm_inode_remove_acl(struct mnt_idmap *idmap,
-				       struct dentry *dentry,
-				       const char *acl_name)
-{
-	return 0;
-}
-
-static inline void evm_inode_post_set_acl(struct dentry *dentry,
-					  const char *acl_name,
-					  struct posix_acl *kacl)
-{
-	return;
-}
-
-static inline int evm_inode_init_security(struct inode *inode, struct inode *dir,
-					  const struct qstr *qstr,
-					  struct xattr *xattrs)
-{
-	return 0;
-}
-
 static inline bool evm_revalidate_status(const char *xattr_name)
 {
 	return false;
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 8b5c472f78b..c45bc97277c 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -19,6 +19,7 @@ 
 #include <linux/xattr.h>
 #include <linux/integrity.h>
 #include <linux/evm.h>
+#include <linux/lsm_hooks.h>
 #include <linux/magic.h>
 #include <linux/posix_acl_xattr.h>
 
@@ -566,9 +567,9 @@  static int evm_protect_xattr(struct mnt_idmap *idmap,
  * userspace from writing HMAC value.  Writing 'security.evm' requires
  * requires CAP_SYS_ADMIN privileges.
  */
-int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
-		       const char *xattr_name, const void *xattr_value,
-		       size_t xattr_value_len, int flags)
+static int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
+			      const char *xattr_name, const void *xattr_value,
+			      size_t xattr_value_len, int flags)
 {
 	const struct evm_ima_xattr_data *xattr_data = xattr_value;
 
@@ -598,8 +599,8 @@  int evm_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
  * Removing 'security.evm' requires CAP_SYS_ADMIN privileges and that
  * the current value is valid.
  */
-int evm_inode_removexattr(struct mnt_idmap *idmap,
-			  struct dentry *dentry, const char *xattr_name)
+static int evm_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry,
+				 const char *xattr_name)
 {
 	/* Policy permits modification of the protected xattrs even though
 	 * there's no HMAC key loaded
@@ -649,9 +650,11 @@  static inline int evm_inode_set_acl_change(struct mnt_idmap *idmap,
  * Prevent modifying posix acls causing the EVM HMAC to be re-calculated
  * and 'security.evm' xattr updated, unless the existing 'security.evm' is
  * valid.
+ *
+ * Return: zero on success, -EPERM on failure.
  */
-int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
-		      const char *acl_name, struct posix_acl *kacl)
+static int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
+			     const char *acl_name, struct posix_acl *kacl)
 {
 	enum integrity_status evm_status;
 
@@ -690,6 +693,24 @@  int evm_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
 	return -EPERM;
 }
 
+/**
+ * evm_inode_remove_acl - Protect the EVM extended attribute from posix acls
+ * @idmap: idmap of the mount
+ * @dentry: pointer to the affected dentry
+ * @acl_name: name of the posix acl
+ *
+ * Prevent removing posix acls causing the EVM HMAC to be re-calculated
+ * and 'security.evm' xattr updated, unless the existing 'security.evm' is
+ * valid.
+ *
+ * Return: zero on success, -EPERM on failure.
+ */
+static int evm_inode_remove_acl(struct mnt_idmap *idmap, struct dentry *dentry,
+				const char *acl_name)
+{
+	return evm_inode_set_acl(idmap, dentry, acl_name, NULL);
+}
+
 static void evm_reset_status(struct inode *inode)
 {
 	struct integrity_iint_cache *iint;
@@ -738,9 +759,11 @@  bool evm_revalidate_status(const char *xattr_name)
  * __vfs_setxattr_noperm().  The caller of which has taken the inode's
  * i_mutex lock.
  */
-void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
-			     const void *xattr_value, size_t xattr_value_len,
-			     int flags)
+static void evm_inode_post_setxattr(struct dentry *dentry,
+				    const char *xattr_name,
+				    const void *xattr_value,
+				    size_t xattr_value_len,
+				    int flags)
 {
 	if (!evm_revalidate_status(xattr_name))
 		return;
@@ -756,6 +779,21 @@  void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
 	evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len);
 }
 
+/**
+ * evm_inode_post_set_acl - Update the EVM extended attribute from posix acls
+ * @dentry: pointer to the affected dentry
+ * @acl_name: name of the posix acl
+ * @kacl: pointer to the posix acls
+ *
+ * Update the 'security.evm' xattr with the EVM HMAC re-calculated after setting
+ * posix acls.
+ */
+static void evm_inode_post_set_acl(struct dentry *dentry, const char *acl_name,
+				   struct posix_acl *kacl)
+{
+	return evm_inode_post_setxattr(dentry, acl_name, NULL, 0, 0);
+}
+
 /**
  * evm_inode_post_removexattr - update 'security.evm' after removing the xattr
  * @dentry: pointer to the affected dentry
@@ -766,7 +804,8 @@  void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
  * No need to take the i_mutex lock here, as this function is called from
  * vfs_removexattr() which takes the i_mutex.
  */
-void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
+static void evm_inode_post_removexattr(struct dentry *dentry,
+				       const char *xattr_name)
 {
 	if (!evm_revalidate_status(xattr_name))
 		return;
@@ -782,6 +821,22 @@  void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
 	evm_update_evmxattr(dentry, xattr_name, NULL, 0);
 }
 
+/**
+ * evm_inode_post_remove_acl - Update the EVM extended attribute from posix acls
+ * @idmap: idmap of the mount
+ * @dentry: pointer to the affected dentry
+ * @acl_name: name of the posix acl
+ *
+ * Update the 'security.evm' xattr with the EVM HMAC re-calculated after
+ * removing posix acls.
+ */
+static inline void evm_inode_post_remove_acl(struct mnt_idmap *idmap,
+					     struct dentry *dentry,
+					     const char *acl_name)
+{
+	evm_inode_post_removexattr(dentry, acl_name);
+}
+
 static int evm_attr_change(struct mnt_idmap *idmap,
 			   struct dentry *dentry, struct iattr *attr)
 {
@@ -805,8 +860,8 @@  static int evm_attr_change(struct mnt_idmap *idmap,
  * Permit update of file attributes when files have a valid EVM signature,
  * except in the case of them having an immutable portable signature.
  */
-int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
-		      struct iattr *attr)
+static int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
+			     struct iattr *attr)
 {
 	unsigned int ia_valid = attr->ia_valid;
 	enum integrity_status evm_status;
@@ -853,8 +908,8 @@  int evm_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
  * This function is called from notify_change(), which expects the caller
  * to lock the inode's i_mutex.
  */
-void evm_inode_post_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
-			    int ia_valid)
+static void evm_inode_post_setattr(struct mnt_idmap *idmap,
+				   struct dentry *dentry, int ia_valid)
 {
 	if (!evm_revalidate_status(NULL))
 		return;
@@ -892,7 +947,7 @@  int evm_inode_init_security(struct inode *inode, struct inode *dir,
 	if (!evm_protected_xattrs)
 		return -EOPNOTSUPP;
 
-	evm_xattr = xattr;
+	evm_xattr = xattrs + integrity_blob_sizes.lbs_xattr;
 
 	xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);
 	if (!xattr_data)
@@ -952,4 +1007,23 @@  static int __init init_evm(void)
 	return error;
 }
 
+static struct security_hook_list evm_hooks[] __lsm_ro_after_init = {
+	LSM_HOOK_INIT(inode_setattr, evm_inode_setattr),
+	LSM_HOOK_INIT(inode_post_setattr, evm_inode_post_setattr),
+	LSM_HOOK_INIT(inode_setxattr, evm_inode_setxattr),
+	LSM_HOOK_INIT(inode_set_acl, evm_inode_set_acl),
+	LSM_HOOK_INIT(inode_post_set_acl, evm_inode_post_set_acl),
+	LSM_HOOK_INIT(inode_remove_acl, evm_inode_remove_acl),
+	LSM_HOOK_INIT(inode_post_remove_acl, evm_inode_post_remove_acl),
+	LSM_HOOK_INIT(inode_post_setxattr, evm_inode_post_setxattr),
+	LSM_HOOK_INIT(inode_removexattr, evm_inode_removexattr),
+	LSM_HOOK_INIT(inode_post_removexattr, evm_inode_post_removexattr),
+	LSM_HOOK_INIT(inode_init_security, evm_inode_init_security),
+};
+
+void __init init_evm_lsm(void)
+{
+	security_add_hooks(evm_hooks, ARRAY_SIZE(evm_hooks), "integrity");
+}
+
 late_initcall(init_evm);
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index bbadf974b31..952d5ea4e18 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -179,12 +179,19 @@  static int __init integrity_lsm_init(void)
 			      0, SLAB_PANIC, init_once);
 
 	init_ima_lsm();
+	init_evm_lsm();
 	return 0;
 }
+
+struct lsm_blob_sizes integrity_blob_sizes __lsm_ro_after_init = {
+	.lbs_xattr = 1,
+};
+
 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 c72d375a356..76e7eda6651 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -188,6 +188,7 @@  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;
 
 struct modsig;
 
@@ -199,6 +200,14 @@  static inline void __init init_ima_lsm(void)
 }
 #endif
 
+#ifdef CONFIG_EVM
+void __init init_evm_lsm(void);
+#else
+static inline void __init init_evm_lsm(void)
+{
+}
+#endif
+
 #ifdef CONFIG_INTEGRITY_SIGNATURE
 
 int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
diff --git a/security/security.c b/security/security.c
index 9bc6a4ef758..74abf04feef 100644
--- a/security/security.c
+++ b/security/security.c
@@ -20,13 +20,13 @@ 
 #include <linux/kernel_read_file.h>
 #include <linux/lsm_hooks.h>
 #include <linux/integrity.h>
-#include <linux/evm.h>
 #include <linux/fsnotify.h>
 #include <linux/mman.h>
 #include <linux/mount.h>
 #include <linux/personality.h>
 #include <linux/backing-dev.h>
 #include <linux/string.h>
+#include <linux/xattr.h>
 #include <linux/msg.h>
 #include <net/flow.h>
 
@@ -1662,8 +1662,8 @@  int security_inode_init_security(struct inode *inode, struct inode *dir,
 	if (!initxattrs)
 		return call_int_hook(inode_init_security, -EOPNOTSUPP, inode,
 				    dir, qstr, NULL);
-	/* Allocate +1 for EVM and +1 as terminator. */
-	new_xattrs = kcalloc(blob_sizes.lbs_xattr + 2, sizeof(*new_xattrs),
+	/* Allocate +1 for terminator. */
+	new_xattrs = kcalloc(blob_sizes.lbs_xattr + 1, sizeof(*new_xattrs),
 			     GFP_NOFS);
 	if (!new_xattrs)
 		return -ENOMEM;
@@ -1699,9 +1699,6 @@  int security_inode_init_security(struct inode *inode, struct inode *dir,
 	if (!num_filled_xattrs)
 		goto out;
 
-	ret = evm_inode_init_security(inode, dir, qstr, new_xattrs);
-	if (ret && ret != -EOPNOTSUPP)
-		goto out;
 	ret = initxattrs(inode, new_xattrs, fs_data);
 out:
 	for (xattr = new_xattrs; xattr->value != NULL; xattr++)
@@ -2201,14 +2198,9 @@  int security_inode_permission(struct inode *inode, int mask)
 int security_inode_setattr(struct mnt_idmap *idmap,
 			   struct dentry *dentry, struct iattr *attr)
 {
-	int ret;
-
 	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
 		return 0;
-	ret = call_int_hook(inode_setattr, 0, idmap, dentry, attr);
-	if (ret)
-		return ret;
-	return evm_inode_setattr(idmap, dentry, attr);
+	return call_int_hook(inode_setattr, 0, idmap, dentry, attr);
 }
 EXPORT_SYMBOL_GPL(security_inode_setattr);
 
@@ -2272,9 +2264,7 @@  int security_inode_setxattr(struct mnt_idmap *idmap,
 
 	if (ret == 1)
 		ret = cap_inode_setxattr(dentry, name, value, size, flags);
-	if (ret)
-		return ret;
-	return evm_inode_setxattr(idmap, dentry, name, value, size, flags);
+	return ret;
 }
 
 /**
@@ -2293,15 +2283,10 @@  int security_inode_set_acl(struct mnt_idmap *idmap,
 			   struct dentry *dentry, const char *acl_name,
 			   struct posix_acl *kacl)
 {
-	int ret;
-
 	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
 		return 0;
-	ret = call_int_hook(inode_set_acl, 0, idmap, dentry, acl_name,
-			    kacl);
-	if (ret)
-		return ret;
-	return evm_inode_set_acl(idmap, dentry, acl_name, kacl);
+	return call_int_hook(inode_set_acl, 0, idmap, dentry, acl_name,
+			     kacl);
 }
 
 /**
@@ -2354,14 +2339,9 @@  int security_inode_get_acl(struct mnt_idmap *idmap,
 int security_inode_remove_acl(struct mnt_idmap *idmap,
 			      struct dentry *dentry, const char *acl_name)
 {
-	int ret;
-
 	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
 		return 0;
-	ret = call_int_hook(inode_remove_acl, 0, idmap, dentry, acl_name);
-	if (ret)
-		return ret;
-	return evm_inode_remove_acl(idmap, dentry, acl_name);
+	return call_int_hook(inode_remove_acl, 0, idmap, dentry, acl_name);
 }
 
 /**
@@ -2397,7 +2377,6 @@  void security_inode_post_setxattr(struct dentry *dentry, const char *name,
 	if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))
 		return;
 	call_void_hook(inode_post_setxattr, dentry, name, value, size, flags);
-	evm_inode_post_setxattr(dentry, name, value, size, flags);
 }
 
 /**
@@ -2458,9 +2437,7 @@  int security_inode_removexattr(struct mnt_idmap *idmap,
 	ret = call_int_hook(inode_removexattr, 1, idmap, dentry, name);
 	if (ret == 1)
 		ret = cap_inode_removexattr(idmap, dentry, name);
-	if (ret)
-		return ret;
-	return evm_inode_removexattr(idmap, dentry, name);
+	return ret;
 }
 
 /**