linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Roberto Sassu <roberto.sassu@huaweicloud.com>
To: viro@zeniv.linux.org.uk, brauner@kernel.org,
	chuck.lever@oracle.com,  jlayton@kernel.org, neilb@suse.de,
	kolga@netapp.com, Dai.Ngo@oracle.com,  tom@talpey.com,
	paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com,
	 zohar@linux.ibm.com, dmitry.kasatkin@gmail.com,
	dhowells@redhat.com,  jarkko@kernel.org,
	stephen.smalley.work@gmail.com, eparis@parisplace.org,
	 casey@schaufler-ca.com, shuah@kernel.org, mic@digikod.net
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	 linux-nfs@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	 linux-integrity@vger.kernel.org, keyrings@vger.kernel.org,
	 selinux@vger.kernel.org, linux-kselftest@vger.kernel.org,
	Roberto Sassu <roberto.sassu@huawei.com>
Subject: Re: [PATCH v8 23/24] ima: Make it independent from 'integrity' LSM
Date: Mon, 18 Dec 2023 09:21:42 +0100	[thread overview]
Message-ID: <4ddaa086a8fe8583e7bfae69fe40daab8c4ac3c1.camel@huaweicloud.com> (raw)
In-Reply-To: <20231214170834.3324559-24-roberto.sassu@huaweicloud.com>

On Thu, 2023-12-14 at 18:08 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> Make the 'ima' LSM independent from the 'integrity' LSM by introducing IMA
> own integrity metadata (ima_iint_cache structure, with IMA-specific fields
> from the integrity_iint_cache structure), and by managing it directly from
> the 'ima' LSM.
> 
> Move the remaining IMA-specific flags to security/integrity/ima/ima.h,
> since they are now unnecessary in the common integrity layer.
> 
> Replace integrity_iint_cache with ima_iint_cache in various places
> of the IMA code.
> 
> Then, reserve space in the security blob for the entire ima_iint_cache
> structure, so that it is available for all inodes having the security blob
> allocated (those for which security_inode_alloc() was called). Adjust the
> IMA code accordingly, call ima_iint_inode() to retrieve the ima_iint_cache
> structure. Keep the non-NULL checks since there can be inodes without
> security blob.
> 
> Don't include the inode pointer as field in the ima_iint_cache structure,
> since the association with the inode is clear. Since the inode field is
> missing in ima_iint_cache, pass the extra inode parameter to
> ima_get_verity_digest().
> 
> Finally, register ima_inode_alloc_security/ima_inode_free_security() to
> initialize/deinitialize the new ima_iint_cache structure (before this task
> was done by iint_init_always() and iint_free()). Also, duplicate
> iint_lockdep_annotate() for the ima_iint_cache structure, and name it
> ima_iint_lockdep_annotate().
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  security/integrity/ima/ima.h          | 109 ++++++++++++++++++++++----
>  security/integrity/ima/ima_api.c      |  15 ++--
>  security/integrity/ima/ima_appraise.c |  25 +++---
>  security/integrity/ima/ima_init.c     |   2 +-
>  security/integrity/ima/ima_main.c     |  96 ++++++++++++++++++-----
>  security/integrity/ima/ima_policy.c   |   2 +-
>  security/integrity/integrity.h        |  53 -------------
>  7 files changed, 194 insertions(+), 108 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index a27fc10f84f7..d1c339a340f8 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -60,7 +60,7 @@ extern const char boot_aggregate_name[];
>  
>  /* IMA event related data */
>  struct ima_event_data {
> -	struct integrity_iint_cache *iint;
> +	struct ima_iint_cache *iint;
>  	struct file *file;
>  	const unsigned char *filename;
>  	struct evm_ima_xattr_data *xattr_value;
> @@ -119,6 +119,86 @@ struct ima_kexec_hdr {
>  	u64 count;
>  };
>  
> +/* IMA iint action cache flags */
> +#define IMA_MEASURE		0x00000001
> +#define IMA_MEASURED		0x00000002
> +#define IMA_APPRAISE		0x00000004
> +#define IMA_APPRAISED		0x00000008
> +/*#define IMA_COLLECT		0x00000010  do not use this flag */
> +#define IMA_COLLECTED		0x00000020
> +#define IMA_AUDIT		0x00000040
> +#define IMA_AUDITED		0x00000080
> +#define IMA_HASH		0x00000100
> +#define IMA_HASHED		0x00000200
> +
> +/* IMA iint policy rule cache flags */
> +#define IMA_NONACTION_FLAGS	0xff000000
> +#define IMA_DIGSIG_REQUIRED	0x01000000
> +#define IMA_PERMIT_DIRECTIO	0x02000000
> +#define IMA_NEW_FILE		0x04000000
> +#define IMA_FAIL_UNVERIFIABLE_SIGS	0x10000000
> +#define IMA_MODSIG_ALLOWED	0x20000000
> +#define IMA_CHECK_BLACKLIST	0x40000000
> +#define IMA_VERITY_REQUIRED	0x80000000
> +
> +#define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
> +				 IMA_HASH | IMA_APPRAISE_SUBMASK)
> +#define IMA_DONE_MASK		(IMA_MEASURED | IMA_APPRAISED | IMA_AUDITED | \
> +				 IMA_HASHED | IMA_COLLECTED | \
> +				 IMA_APPRAISED_SUBMASK)
> +
> +/* IMA iint subaction appraise cache flags */
> +#define IMA_FILE_APPRAISE	0x00001000
> +#define IMA_FILE_APPRAISED	0x00002000
> +#define IMA_MMAP_APPRAISE	0x00004000
> +#define IMA_MMAP_APPRAISED	0x00008000
> +#define IMA_BPRM_APPRAISE	0x00010000
> +#define IMA_BPRM_APPRAISED	0x00020000
> +#define IMA_READ_APPRAISE	0x00040000
> +#define IMA_READ_APPRAISED	0x00080000
> +#define IMA_CREDS_APPRAISE	0x00100000
> +#define IMA_CREDS_APPRAISED	0x00200000
> +#define IMA_APPRAISE_SUBMASK	(IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \
> +				 IMA_BPRM_APPRAISE | IMA_READ_APPRAISE | \
> +				 IMA_CREDS_APPRAISE)
> +#define IMA_APPRAISED_SUBMASK	(IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \
> +				 IMA_BPRM_APPRAISED | IMA_READ_APPRAISED | \
> +				 IMA_CREDS_APPRAISED)
> +
> +/* IMA iint cache atomic_flags */
> +#define IMA_CHANGE_XATTR	0
> +#define IMA_UPDATE_XATTR	1
> +#define IMA_CHANGE_ATTR		2
> +#define IMA_DIGSIG		3
> +#define IMA_MUST_MEASURE	4
> +
> +/* IMA integrity metadata associated with an inode */
> +struct ima_iint_cache {
> +	struct mutex mutex;	/* protects: version, flags, digest */
> +	u64 version;		/* track inode changes */
> +	unsigned long flags;
> +	unsigned long measured_pcrs;
> +	unsigned long atomic_flags;
> +	unsigned long real_ino;
> +	dev_t real_dev;
> +	enum integrity_status ima_file_status:4;
> +	enum integrity_status ima_mmap_status:4;
> +	enum integrity_status ima_bprm_status:4;
> +	enum integrity_status ima_read_status:4;
> +	enum integrity_status ima_creds_status:4;
> +	struct ima_digest_data *ima_hash;
> +};
> +
> +extern struct lsm_blob_sizes ima_blob_sizes;
> +
> +static inline struct ima_iint_cache *ima_iint_inode(const struct inode *inode)
> +{
> +	if (unlikely(!inode->i_security))
> +		return NULL;
> +
> +	return inode->i_security + ima_blob_sizes.lbs_inode;
> +}
> +
>  extern const int read_idmap[];
>  
>  #ifdef CONFIG_HAVE_IMA_KEXEC
> @@ -152,7 +232,7 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
>  			      struct ima_template_entry *entry);
>  int ima_calc_boot_aggregate(struct ima_digest_data *hash);
>  void ima_add_violation(struct file *file, const unsigned char *filename,
> -		       struct integrity_iint_cache *iint,
> +		       struct ima_iint_cache *iint,
>  		       const char *op, const char *cause);
>  int ima_init_crypto(void);
>  void ima_putc(struct seq_file *m, void *data, int datalen);
> @@ -267,10 +347,10 @@ int ima_get_action(struct mnt_idmap *idmap, struct inode *inode,
>  		   struct ima_template_desc **template_desc,
>  		   const char *func_data, unsigned int *allowed_algos);
>  int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
> -int ima_collect_measurement(struct integrity_iint_cache *iint,
> +int ima_collect_measurement(struct ima_iint_cache *iint,
>  			    struct file *file, void *buf, loff_t size,
>  			    enum hash_algo algo, struct modsig *modsig);
> -void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
> +void ima_store_measurement(struct ima_iint_cache *iint, struct file *file,
>  			   const unsigned char *filename,
>  			   struct evm_ima_xattr_data *xattr_value,
>  			   int xattr_len, const struct modsig *modsig, int pcr,
> @@ -280,7 +360,7 @@ int process_buffer_measurement(struct mnt_idmap *idmap,
>  			       const char *eventname, enum ima_hooks func,
>  			       int pcr, const char *func_data,
>  			       bool buf_hash, u8 *digest, size_t digest_len);
> -void ima_audit_measurement(struct integrity_iint_cache *iint,
> +void ima_audit_measurement(struct ima_iint_cache *iint,
>  			   const unsigned char *filename);
>  int ima_alloc_init_template(struct ima_event_data *event_data,
>  			    struct ima_template_entry **entry,
> @@ -318,17 +398,17 @@ int ima_policy_show(struct seq_file *m, void *v);
>  #define IMA_APPRAISE_KEXEC	0x40
>  
>  #ifdef CONFIG_IMA_APPRAISE
> -int ima_check_blacklist(struct integrity_iint_cache *iint,
> +int ima_check_blacklist(struct ima_iint_cache *iint,
>  			const struct modsig *modsig, int pcr);
>  int ima_appraise_measurement(enum ima_hooks func,
> -			     struct integrity_iint_cache *iint,
> +			     struct ima_iint_cache *iint,
>  			     struct file *file, const unsigned char *filename,
>  			     struct evm_ima_xattr_data *xattr_value,
>  			     int xattr_len, const struct modsig *modsig);
>  int ima_must_appraise(struct mnt_idmap *idmap, struct inode *inode,
>  		      int mask, enum ima_hooks func);
> -void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
> -enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
> +void ima_update_xattr(struct ima_iint_cache *iint, struct file *file);
> +enum integrity_status ima_get_cache_status(struct ima_iint_cache *iint,
>  					   enum ima_hooks func);
>  enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value,
>  				 int xattr_len);
> @@ -337,14 +417,14 @@ int ima_read_xattr(struct dentry *dentry,
>  void __init init_ima_appraise_lsm(const struct lsm_id *lsmid);
>  
>  #else
> -static inline int ima_check_blacklist(struct integrity_iint_cache *iint,
> +static inline int ima_check_blacklist(struct ima_iint_cache *iint,
>  				      const struct modsig *modsig, int pcr)
>  {
>  	return 0;
>  }
>  
>  static inline int ima_appraise_measurement(enum ima_hooks func,
> -					   struct integrity_iint_cache *iint,
> +					   struct ima_iint_cache *iint,
>  					   struct file *file,
>  					   const unsigned char *filename,
>  					   struct evm_ima_xattr_data *xattr_value,
> @@ -361,14 +441,13 @@ static inline int ima_must_appraise(struct mnt_idmap *idmap,
>  	return 0;
>  }
>  
> -static inline void ima_update_xattr(struct integrity_iint_cache *iint,
> +static inline void ima_update_xattr(struct ima_iint_cache *iint,
>  				    struct file *file)
>  {
>  }
>  
> -static inline enum integrity_status ima_get_cache_status(struct integrity_iint_cache
> -							 *iint,
> -							 enum ima_hooks func)
> +static inline enum integrity_status
> +ima_get_cache_status(struct ima_iint_cache *iint, enum ima_hooks func)
>  {
>  	return INTEGRITY_UNKNOWN;
>  }
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index 597ea0c4d72f..f3363935804f 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -131,7 +131,7 @@ int ima_store_template(struct ima_template_entry *entry,
>   * value is invalidated.
>   */
>  void ima_add_violation(struct file *file, const unsigned char *filename,
> -		       struct integrity_iint_cache *iint,
> +		       struct ima_iint_cache *iint,
>  		       const char *op, const char *cause)
>  {
>  	struct ima_template_entry *entry;
> @@ -201,7 +201,8 @@ int ima_get_action(struct mnt_idmap *idmap, struct inode *inode,
>  				allowed_algos);
>  }
>  
> -static bool ima_get_verity_digest(struct integrity_iint_cache *iint,
> +static bool ima_get_verity_digest(struct ima_iint_cache *iint,
> +				  struct inode *inode,
>  				  struct ima_max_digest_data *hash)
>  {
>  	enum hash_algo alg;
> @@ -211,7 +212,7 @@ static bool ima_get_verity_digest(struct integrity_iint_cache *iint,
>  	 * On failure, 'measure' policy rules will result in a file data
>  	 * hash containing 0's.
>  	 */
> -	digest_len = fsverity_get_digest(iint->inode, hash->digest, NULL, &alg);
> +	digest_len = fsverity_get_digest(inode, hash->digest, NULL, &alg);
>  	if (digest_len == 0)
>  		return false;
>  
> @@ -237,7 +238,7 @@ static bool ima_get_verity_digest(struct integrity_iint_cache *iint,
>   *
>   * Return 0 on success, error code otherwise
>   */
> -int ima_collect_measurement(struct integrity_iint_cache *iint,
> +int ima_collect_measurement(struct ima_iint_cache *iint,
>  			    struct file *file, void *buf, loff_t size,
>  			    enum hash_algo algo, struct modsig *modsig)
>  {
> @@ -280,7 +281,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
>  	memset(&hash.digest, 0, sizeof(hash.digest));
>  
>  	if (iint->flags & IMA_VERITY_REQUIRED) {
> -		if (!ima_get_verity_digest(iint, &hash)) {
> +		if (!ima_get_verity_digest(iint, inode, &hash)) {
>  			audit_cause = "no-verity-digest";
>  			result = -ENODATA;
>  		}
> @@ -338,7 +339,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
>   *
>   * Must be called with iint->mutex held.
>   */
> -void ima_store_measurement(struct integrity_iint_cache *iint,
> +void ima_store_measurement(struct ima_iint_cache *iint,
>  			   struct file *file, const unsigned char *filename,
>  			   struct evm_ima_xattr_data *xattr_value,
>  			   int xattr_len, const struct modsig *modsig, int pcr,
> @@ -382,7 +383,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
>  		ima_free_template_entry(entry);
>  }
>  
> -void ima_audit_measurement(struct integrity_iint_cache *iint,
> +void ima_audit_measurement(struct ima_iint_cache *iint,
>  			   const unsigned char *filename)
>  {
>  	struct audit_buffer *ab;
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 1dd6ee72a20a..d71df7deacb7 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -85,7 +85,7 @@ int ima_must_appraise(struct mnt_idmap *idmap, struct inode *inode,
>  }
>  
>  static int ima_fix_xattr(struct dentry *dentry,
> -			 struct integrity_iint_cache *iint)
> +			 struct ima_iint_cache *iint)
>  {
>  	int rc, offset;
>  	u8 algo = iint->ima_hash->algo;
> @@ -106,7 +106,7 @@ static int ima_fix_xattr(struct dentry *dentry,
>  }
>  
>  /* Return specific func appraised cached result */
> -enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
> +enum integrity_status ima_get_cache_status(struct ima_iint_cache *iint,
>  					   enum ima_hooks func)
>  {
>  	switch (func) {
> @@ -126,7 +126,7 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
>  	}
>  }
>  
> -static void ima_set_cache_status(struct integrity_iint_cache *iint,
> +static void ima_set_cache_status(struct ima_iint_cache *iint,
>  				 enum ima_hooks func,
>  				 enum integrity_status status)
>  {
> @@ -152,8 +152,7 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint,
>  	}
>  }
>  
> -static void ima_cache_flags(struct integrity_iint_cache *iint,
> -			     enum ima_hooks func)
> +static void ima_cache_flags(struct ima_iint_cache *iint, enum ima_hooks func)
>  {
>  	switch (func) {
>  	case MMAP_CHECK:
> @@ -276,7 +275,7 @@ static int calc_file_id_hash(enum evm_ima_xattr_type type,
>   *
>   * Return 0 on success, error code otherwise.
>   */
> -static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
> +static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint,
>  			struct evm_ima_xattr_data *xattr_value, int xattr_len,
>  			enum integrity_status *status, const char **cause)
>  {
> @@ -443,7 +442,7 @@ static int modsig_verify(enum ima_hooks func, const struct modsig *modsig,
>   *
>   * Returns -EPERM if the hash is blacklisted.
>   */
> -int ima_check_blacklist(struct integrity_iint_cache *iint,
> +int ima_check_blacklist(struct ima_iint_cache *iint,
>  			const struct modsig *modsig, int pcr)
>  {
>  	enum hash_algo hash_algo;
> @@ -478,7 +477,7 @@ int ima_check_blacklist(struct integrity_iint_cache *iint,
>   * Return 0 on success, error code otherwise
>   */
>  int ima_appraise_measurement(enum ima_hooks func,
> -			     struct integrity_iint_cache *iint,
> +			     struct ima_iint_cache *iint,
>  			     struct file *file, const unsigned char *filename,
>  			     struct evm_ima_xattr_data *xattr_value,
>  			     int xattr_len, const struct modsig *modsig)
> @@ -603,7 +602,7 @@ int ima_appraise_measurement(enum ima_hooks func,
>  /*
>   * ima_update_xattr - update 'security.ima' hash value
>   */
> -void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file)
> +void ima_update_xattr(struct ima_iint_cache *iint, struct file *file)
>  {
>  	struct dentry *dentry = file_dentry(file);
>  	int rc = 0;
> @@ -640,7 +639,7 @@ static void ima_inode_post_setattr(struct mnt_idmap *idmap,
>  				   struct dentry *dentry, int ia_valid)
>  {
>  	struct inode *inode = d_backing_inode(dentry);
> -	struct integrity_iint_cache *iint;
> +	struct ima_iint_cache *iint;
>  	int action;
>  
>  	if (!(ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode)
> @@ -648,7 +647,7 @@ static void ima_inode_post_setattr(struct mnt_idmap *idmap,
>  		return;
>  
>  	action = ima_must_appraise(idmap, inode, MAY_ACCESS, POST_SETATTR);
> -	iint = integrity_iint_find(inode);
> +	iint = ima_iint_inode(inode);
>  	if (iint) {
>  		set_bit(IMA_CHANGE_ATTR, &iint->atomic_flags);
>  		if (!action)
> @@ -674,12 +673,12 @@ static int ima_protect_xattr(struct dentry *dentry, const char *xattr_name,
>  
>  static void ima_reset_appraise_flags(struct inode *inode, int digsig)
>  {
> -	struct integrity_iint_cache *iint;
> +	struct ima_iint_cache *iint;
>  
>  	if (!(ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode))
>  		return;
>  
> -	iint = integrity_iint_find(inode);
> +	iint = ima_iint_inode(inode);
>  	if (!iint)
>  		return;
>  	iint->measured_pcrs = 0;
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index 63979aefc95f..393f5c7912d5 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -44,7 +44,7 @@ static int __init ima_add_boot_aggregate(void)
>  	static const char op[] = "add_boot_aggregate";
>  	const char *audit_cause = "ENOMEM";
>  	struct ima_template_entry *entry;
> -	struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
> +	struct ima_iint_cache tmp_iint, *iint = &tmp_iint;
>  	struct ima_event_data event_data = { .iint = iint,
>  					     .filename = boot_aggregate_name };
>  	struct ima_max_digest_data hash;
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 2a9ca5fa4317..b28e49f53ca4 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -114,7 +114,7 @@ static int mmap_violation_check(enum ima_hooks func, struct file *file,
>   *
>   */
>  static void ima_rdwr_violation_check(struct file *file,
> -				     struct integrity_iint_cache *iint,
> +				     struct ima_iint_cache *iint,
>  				     int must_measure,
>  				     char **pathbuf,
>  				     const char **pathname,
> @@ -125,9 +125,9 @@ static void ima_rdwr_violation_check(struct file *file,
>  	bool send_tomtou = false, send_writers = false;
>  
>  	if (mode & FMODE_WRITE) {
> -		if (atomic_read(&inode->i_readcount) && IS_IMA(inode)) {
> +		if (atomic_read(&inode->i_readcount)) {
>  			if (!iint)
> -				iint = integrity_iint_find(inode);
> +				iint = ima_iint_inode(inode);
>  			/* IMA_MEASURE is set from reader side */
>  			if (iint && test_bit(IMA_MUST_MEASURE,
>  						&iint->atomic_flags))
> @@ -153,7 +153,7 @@ static void ima_rdwr_violation_check(struct file *file,
>  				  "invalid_pcr", "open_writers");
>  }
>  
> -static void ima_check_last_writer(struct integrity_iint_cache *iint,
> +static void ima_check_last_writer(struct ima_iint_cache *iint,
>  				  struct inode *inode, struct file *file)
>  {
>  	fmode_t mode = file->f_mode;
> @@ -192,12 +192,12 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint,
>  static void ima_file_free(struct file *file)
>  {
>  	struct inode *inode = file_inode(file);
> -	struct integrity_iint_cache *iint;
> +	struct ima_iint_cache *iint;
>  
>  	if (!ima_policy_flag || !S_ISREG(inode->i_mode))
>  		return;
>  
> -	iint = integrity_iint_find(inode);
> +	iint = ima_iint_inode(inode);
>  	if (!iint)
>  		return;
>  
> @@ -209,7 +209,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
>  			       enum ima_hooks func)
>  {
>  	struct inode *backing_inode, *inode = file_inode(file);
> -	struct integrity_iint_cache *iint = NULL;
> +	struct ima_iint_cache *iint;

Argh, really sorry...

Last minute change after restoring the non-NULL checks, it seems I
didn't revert all the changes completely (missing = NULL).

>  	struct ima_template_desc *template_desc = NULL;
>  	char *pathbuf = NULL;
>  	char filename[NAME_MAX];
> @@ -248,7 +248,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
>  	inode_lock(inode);
>  
>  	if (action) {
> -		iint = integrity_inode_get(inode);
> +		iint = ima_iint_inode(inode);
>  		if (!iint)
>  			rc = -ENOMEM;
>  	}
> @@ -564,11 +564,11 @@ static int ima_file_check(struct file *file, int mask)
>  static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf,
>  			    size_t buf_size)
>  {
> -	struct integrity_iint_cache *iint = NULL, tmp_iint;
> +	struct ima_iint_cache *iint, tmp_iint;

Same here. Will send v9 after I get more reviews.

Thanks

Roberto

>  	int rc, hash_algo;
>  
>  	if (ima_policy_flag) {
> -		iint = integrity_iint_find(inode);
> +		iint = ima_iint_inode(inode);
>  		if (iint)
>  			mutex_lock(&iint->mutex);
>  	}
> @@ -578,7 +578,6 @@ static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf,
>  			mutex_unlock(&iint->mutex);
>  
>  		memset(&tmp_iint, 0, sizeof(tmp_iint));
> -		tmp_iint.inode = inode;
>  		mutex_init(&tmp_iint.mutex);
>  
>  		rc = ima_collect_measurement(&tmp_iint, file, NULL, 0,
> @@ -688,7 +687,7 @@ static void ima_post_create_tmpfile(struct mnt_idmap *idmap,
>  				    struct inode *inode)
>  
>  {
> -	struct integrity_iint_cache *iint;
> +	struct ima_iint_cache *iint;
>  	int must_appraise;
>  
>  	if (!ima_policy_flag || !S_ISREG(inode->i_mode))
> @@ -699,8 +698,8 @@ static void ima_post_create_tmpfile(struct mnt_idmap *idmap,
>  	if (!must_appraise)
>  		return;
>  
> -	/* Nothing to do if we can't allocate memory */
> -	iint = integrity_inode_get(inode);
> +	iint = ima_iint_inode(inode);
> +	/* iint is NULL if security_inode_alloc() was not called on inode. */
>  	if (!iint)
>  		return;
>  
> @@ -720,8 +719,8 @@ static void ima_post_create_tmpfile(struct mnt_idmap *idmap,
>  static void __maybe_unused
>  ima_post_path_mknod(struct mnt_idmap *idmap, struct dentry *dentry)
>  {
> -	struct integrity_iint_cache *iint;
>  	struct inode *inode = dentry->d_inode;
> +	struct ima_iint_cache *iint = ima_iint_inode(inode);
>  	int must_appraise;
>  
>  	if (!ima_policy_flag || !S_ISREG(inode->i_mode))
> @@ -732,8 +731,8 @@ ima_post_path_mknod(struct mnt_idmap *idmap, struct dentry *dentry)
>  	if (!must_appraise)
>  		return;
>  
> -	/* Nothing to do if we can't allocate memory */
> -	iint = integrity_inode_get(inode);
> +	iint = ima_iint_inode(inode);
> +	/* iint is NULL if security_inode_alloc() was not called on inode. */
>  	if (!iint)
>  		return;
>  
> @@ -936,7 +935,7 @@ int process_buffer_measurement(struct mnt_idmap *idmap,
>  	int ret = 0;
>  	const char *audit_cause = "ENOMEM";
>  	struct ima_template_entry *entry = NULL;
> -	struct integrity_iint_cache iint = {};
> +	struct ima_iint_cache iint = {};
>  	struct ima_event_data event_data = {.iint = &iint,
>  					    .filename = eventname,
>  					    .buf = buf,
> @@ -1145,6 +1144,60 @@ static int __maybe_unused ima_kernel_module_request(char *kmod_name)
>  	return 0;
>  }
>  
> +#define IMA_MAX_NESTING (FILESYSTEM_MAX_STACK_DEPTH + 1)
> +
> +/*
> + * It is not clear that IMA should be nested at all, but as long is it measures
> + * files both on overlayfs and on underlying fs, we need to annotate the iint
> + * mutex to avoid lockdep false positives related to IMA + overlayfs.
> + * See ovl_lockdep_annotate_inode_mutex_key() for more details.
> + */
> +static inline void ima_iint_lockdep_annotate(struct ima_iint_cache *ima_iint,
> +					     struct inode *inode)
> +{
> +#ifdef CONFIG_LOCKDEP
> +	static struct lock_class_key iint_mutex_key[IMA_MAX_NESTING];
> +
> +	int depth = inode->i_sb->s_stack_depth;
> +
> +	if (WARN_ON_ONCE(depth < 0 || depth >= IMA_MAX_NESTING))
> +		depth = 0;
> +
> +	lockdep_set_class(&ima_iint->mutex, &iint_mutex_key[depth]);
> +#endif
> +}
> +
> +static int ima_inode_alloc_security(struct inode *inode)
> +{
> +	struct ima_iint_cache *iint = ima_iint_inode(inode);
> +
> +	/* Called by security_inode_alloc(), it cannot be NULL. */
> +	iint->ima_hash = NULL;
> +	iint->version = 0;
> +	iint->flags = 0UL;
> +	iint->atomic_flags = 0UL;
> +	iint->ima_file_status = INTEGRITY_UNKNOWN;
> +	iint->ima_mmap_status = INTEGRITY_UNKNOWN;
> +	iint->ima_bprm_status = INTEGRITY_UNKNOWN;
> +	iint->ima_read_status = INTEGRITY_UNKNOWN;
> +	iint->ima_creds_status = INTEGRITY_UNKNOWN;
> +	iint->measured_pcrs = 0;
> +	mutex_init(&iint->mutex);
> +	ima_iint_lockdep_annotate(iint, inode);
> +	return 0;
> +}
> +
> +static void ima_inode_free_security(struct inode *inode)
> +{
> +	struct ima_iint_cache *iint = ima_iint_inode(inode);
> +
> +	if (!iint)
> +		return;
> +
> +	kfree(iint->ima_hash);
> +	mutex_destroy(&iint->mutex);
> +}
> +
>  static struct security_hook_list ima_hooks[] __ro_after_init = {
>  	LSM_HOOK_INIT(bprm_check_security, ima_bprm_check),
>  	LSM_HOOK_INIT(file_post_open, ima_file_check),
> @@ -1156,6 +1209,8 @@ static struct security_hook_list ima_hooks[] __ro_after_init = {
>  	LSM_HOOK_INIT(kernel_post_load_data, ima_post_load_data),
>  	LSM_HOOK_INIT(kernel_read_file, ima_read_file),
>  	LSM_HOOK_INIT(kernel_post_read_file, ima_post_read_file),
> +	LSM_HOOK_INIT(inode_alloc_security, ima_inode_alloc_security),
> +	LSM_HOOK_INIT(inode_free_security, ima_inode_free_security),
>  #ifdef CONFIG_SECURITY_PATH
>  	LSM_HOOK_INIT(path_post_mknod, ima_post_path_mknod),
>  #endif
> @@ -1179,10 +1234,15 @@ static int __init init_ima_lsm(void)
>  	return 0;
>  }
>  
> +struct lsm_blob_sizes ima_blob_sizes __ro_after_init = {
> +	.lbs_inode = sizeof(struct ima_iint_cache),
> +};
> +
>  DEFINE_LSM(ima) = {
>  	.name = "ima",
>  	.init = init_ima_lsm,
>  	.order = LSM_ORDER_LAST,
> +	.blobs = &ima_blob_sizes,
>  };
>  
>  late_initcall(init_ima);	/* Start IMA after the TPM is available */
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index f69062617754..c0556907c2e6 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -49,7 +49,7 @@
>  #define DONT_HASH	0x0200
>  
>  #define INVALID_PCR(a) (((a) < 0) || \
> -	(a) >= (sizeof_field(struct integrity_iint_cache, measured_pcrs) * 8))
> +	(a) >= (sizeof_field(struct ima_iint_cache, measured_pcrs) * 8))
>  
>  int ima_policy_flag;
>  static int temp_ima_appraise;
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 7a97c269a072..671fc50255f9 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -20,59 +20,6 @@
>  #include <linux/audit.h>
>  #include <linux/lsm_hooks.h>
>  
> -/* iint action cache flags */
> -#define IMA_MEASURE		0x00000001
> -#define IMA_MEASURED		0x00000002
> -#define IMA_APPRAISE		0x00000004
> -#define IMA_APPRAISED		0x00000008
> -/*#define IMA_COLLECT		0x00000010  do not use this flag */
> -#define IMA_COLLECTED		0x00000020
> -#define IMA_AUDIT		0x00000040
> -#define IMA_AUDITED		0x00000080
> -#define IMA_HASH		0x00000100
> -#define IMA_HASHED		0x00000200
> -
> -/* iint policy rule cache flags */
> -#define IMA_NONACTION_FLAGS	0xff000000
> -#define IMA_DIGSIG_REQUIRED	0x01000000
> -#define IMA_PERMIT_DIRECTIO	0x02000000
> -#define IMA_NEW_FILE		0x04000000
> -#define IMA_FAIL_UNVERIFIABLE_SIGS	0x10000000
> -#define IMA_MODSIG_ALLOWED	0x20000000
> -#define IMA_CHECK_BLACKLIST	0x40000000
> -#define IMA_VERITY_REQUIRED	0x80000000
> -
> -#define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
> -				 IMA_HASH | IMA_APPRAISE_SUBMASK)
> -#define IMA_DONE_MASK		(IMA_MEASURED | IMA_APPRAISED | IMA_AUDITED | \
> -				 IMA_HASHED | IMA_COLLECTED | \
> -				 IMA_APPRAISED_SUBMASK)
> -
> -/* iint subaction appraise cache flags */
> -#define IMA_FILE_APPRAISE	0x00001000
> -#define IMA_FILE_APPRAISED	0x00002000
> -#define IMA_MMAP_APPRAISE	0x00004000
> -#define IMA_MMAP_APPRAISED	0x00008000
> -#define IMA_BPRM_APPRAISE	0x00010000
> -#define IMA_BPRM_APPRAISED	0x00020000
> -#define IMA_READ_APPRAISE	0x00040000
> -#define IMA_READ_APPRAISED	0x00080000
> -#define IMA_CREDS_APPRAISE	0x00100000
> -#define IMA_CREDS_APPRAISED	0x00200000
> -#define IMA_APPRAISE_SUBMASK	(IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \
> -				 IMA_BPRM_APPRAISE | IMA_READ_APPRAISE | \
> -				 IMA_CREDS_APPRAISE)
> -#define IMA_APPRAISED_SUBMASK	(IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \
> -				 IMA_BPRM_APPRAISED | IMA_READ_APPRAISED | \
> -				 IMA_CREDS_APPRAISED)
> -
> -/* iint cache atomic_flags */
> -#define IMA_CHANGE_XATTR	0
> -#define IMA_UPDATE_XATTR	1
> -#define IMA_CHANGE_ATTR		2
> -#define IMA_DIGSIG		3
> -#define IMA_MUST_MEASURE	4
> -
>  enum evm_ima_xattr_type {
>  	IMA_XATTR_DIGEST = 0x01,
>  	EVM_XATTR_HMAC,


  reply	other threads:[~2023-12-18  8:39 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-14 17:08 [PATCH v8 00/24] security: Move IMA and EVM to the LSM infrastructure Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 01/24] ima: Align ima_inode_post_setattr() definition with " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 02/24] ima: Align ima_file_mprotect() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 03/24] ima: Align ima_inode_setxattr() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 04/24] ima: Align ima_inode_removexattr() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 05/24] ima: Align ima_post_read_file() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 06/24] evm: Align evm_inode_post_setattr() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 07/24] evm: Align evm_inode_setxattr() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 08/24] evm: Align evm_inode_post_setxattr() " Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 09/24] security: Align inode_setattr hook definition with EVM Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 10/24] security: Introduce inode_post_setattr hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 11/24] security: Introduce inode_post_removexattr hook Roberto Sassu
2023-12-15 21:05   ` Casey Schaufler
2023-12-14 17:08 ` [PATCH v8 12/24] security: Introduce file_post_open hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 13/24] security: Introduce file_release hook Roberto Sassu
2023-12-15 21:41   ` Casey Schaufler
2023-12-14 17:08 ` [PATCH v8 14/24] security: Introduce path_post_mknod hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 15/24] security: Introduce inode_post_create_tmpfile hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 16/24] security: Introduce inode_post_set_acl hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 17/24] security: Introduce inode_post_remove_acl hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 18/24] security: Introduce key_post_create_or_update hook Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 19/24] ima: Move to LSM infrastructure Roberto Sassu
2023-12-26 18:14   ` Mimi Zohar
2023-12-26 20:14     ` Casey Schaufler
2023-12-27 19:52       ` Mimi Zohar
2023-12-27 20:20         ` Casey Schaufler
2023-12-14 17:08 ` [PATCH v8 20/24] ima: Move IMA-Appraisal " Roberto Sassu
2023-12-26 22:15   ` Mimi Zohar
2023-12-14 17:08 ` [PATCH v8 21/24] evm: Move " Roberto Sassu
2023-12-15 21:54   ` Casey Schaufler
2023-12-26 22:13   ` Mimi Zohar
2024-01-02 11:56     ` Roberto Sassu
2024-01-02 17:44       ` Mimi Zohar
2023-12-14 17:08 ` [PATCH v8 22/24] evm: Make it independent from 'integrity' LSM Roberto Sassu
2023-12-27  3:12   ` Mimi Zohar
2023-12-14 17:08 ` [PATCH v8 23/24] ima: " Roberto Sassu
2023-12-18  8:21   ` Roberto Sassu [this message]
2023-12-27 13:22   ` Mimi Zohar
2023-12-27 16:39     ` Roberto Sassu
2023-12-27 19:21       ` Mimi Zohar
2024-01-02 10:53         ` Roberto Sassu
2023-12-14 17:08 ` [PATCH v8 24/24] integrity: Remove LSM Roberto Sassu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4ddaa086a8fe8583e7bfae69fe40daab8c4ac3c1.camel@huaweicloud.com \
    --to=roberto.sassu@huaweicloud.com \
    --cc=Dai.Ngo@oracle.com \
    --cc=brauner@kernel.org \
    --cc=casey@schaufler-ca.com \
    --cc=chuck.lever@oracle.com \
    --cc=dhowells@redhat.com \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=eparis@parisplace.org \
    --cc=jarkko@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=jmorris@namei.org \
    --cc=keyrings@vger.kernel.org \
    --cc=kolga@netapp.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mic@digikod.net \
    --cc=neilb@suse.de \
    --cc=paul@paul-moore.com \
    --cc=roberto.sassu@huawei.com \
    --cc=selinux@vger.kernel.org \
    --cc=serge@hallyn.com \
    --cc=shuah@kernel.org \
    --cc=stephen.smalley.work@gmail.com \
    --cc=tom@talpey.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).