Re: [RFC][PATCH 8/8] ima: Detect if digest cache changed since last measurement/appraisal

[Mimi Zohar <zohar@xxxxxxxxxxxxx>]

Hi Roberto,

> b/security/integrity/ima/ima_main.c
> index a66522a22cbc..e1b2f5737753 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -301,6 +301,15 @@ static int process_measurement(struct file *file, const
> struct cred *cred,
>  		}
>  	}
>  
> +	/* Check if digest cache changed since last measurement/appraisal. */
> +	if (iint->digest_cache &&
> +	    digest_cache_changed(inode, iint->digest_cache)) {
> +		iint->flags &= ~IMA_DONE_MASK;
> +		iint->measured_pcrs = 0;
> +		digest_cache_put(iint->digest_cache);
> +		iint->digest_cache = NULL;
> +	}
> +
>  	/* Determine if already appraised/measured based on bitmask
>  	 * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED,
>  	 *  IMA_AUDIT, IMA_AUDITED)
> @@ -371,8 +380,15 @@ static int process_measurement(struct file *file, const
> struct cred *cred,
>  	 * Since we allow IMA policy rules without func=, we have to enforce
>  	 * this restriction here.
>  	 */
> -	if (rc == 0 && policy_mask && func != DIGEST_LIST_CHECK)
> -		digest_cache = digest_cache_get(file_dentry(file));
> +	if (rc == 0 && policy_mask && func != DIGEST_LIST_CHECK) {
> +		if (!iint->digest_cache) {
> +			/* Released by ima_iint_free(). */
> +			digest_cache = digest_cache_get(file_dentry(file));
> +			iint->digest_cache = digest_cache;
> +		} else {
> +			digest_cache = iint->digest_cache;
> +		}

Simple cleanup:
		if (!iint->digest_cache)
			iint->digest_cache =digest_cache_get(file_dentry(file));

		digest_cache = iint->digest_cache;

> +	}
>  
>  	if (digest_cache) {
>  		found = digest_cache_lookup(file_dentry(file), digest_cache,
> @@ -386,8 +402,6 @@ static int process_measurement(struct file *file, const
> struct cred *cred,
>  			if (verif_mask_ptr)
>  				allow_mask = policy_mask & *verif_mask_ptr;
>  		}
> -
> -		digest_cache_put(digest_cache);

Keeping a reference to the digest_cache list for each file in the iint cache
until the file is re-accessed, might take a while to free.

I'm wondering if it necessary to keep a reference to the digest_cache.  Or is it
possible to just compare the existing iint->digest_cache pointer with the
current digest_cache pointer?

thanks,

Mimi

>  	}
>  
>  	if (action & IMA_MEASURE)