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

[Mimi Zohar]

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)