Hi Roberto, > diff --git a/security/integrity/ima/ima_main.c > b/security/integrity/ima/ima_main.c > index 3fc48214850a..48a09747ae7a 100644 > --- a/security/integrity/ima/ima_main.c > +++ b/security/integrity/ima/ima_main.c > @@ -222,7 +222,9 @@ static int process_measurement(struct file *file, const > struct cred *cred, > bool violation_check; > enum hash_algo hash_algo; > unsigned int allowed_algos = 0; > - u64 verif_mask = 0; > + u64 verif_mask = 0, *verif_mask_ptr, policy_mask = 0, allow_mask = 0; > + struct digest_cache *digest_cache = NULL, *found_cache; > + digest_cache_found_t found; > > if (!ima_policy_flag || !S_ISREG(inode->i_mode)) > return 0; > @@ -233,7 +235,7 @@ static int process_measurement(struct file *file, const > struct cred *cred, > */ > action = ima_get_action(file_mnt_idmap(file), inode, cred, secid, > mask, func, &pcr, &template_desc, NULL, > - &allowed_algos, NULL); > + &allowed_algos, &policy_mask); > violation_check = ((func == FILE_CHECK || func == MMAP_CHECK || > func == MMAP_CHECK_REQPROT) && > (ima_policy_flag & IMA_MEASURE)); > @@ -364,10 +366,34 @@ static int process_measurement(struct file *file, const > struct cred *cred, > if (!pathbuf) /* ima_rdwr_violation possibly pre-fetched */ > pathname = ima_d_path(&file->f_path, &pathbuf, filename); > > + /* > + * For now we don't support nested verification with digest caches.
I haven't reviewed the digest_cache LSM patch set yet. What does 'nested' mean in this context? Why mention it here? > + * 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)); So whether or not a DIGEST_LIST_CHECK policy rule even exists, digest_cache_get() will be called. Similarly, even if a digest_cache list hasn't been measured or appraised, digest_cache_get() will be called. Basically every file in policy will check the digest_cache. > + > + if (digest_cache) { > + found = digest_cache_lookup(file_dentry(file), digest_cache, > + iint->ima_hash->digest, > + iint->ima_hash->algo); > + /* AND what is allowed by the policy, and what IMA verified. */ > + if (found) { > + found_cache = digest_cache_from_found_t(found); > + verif_mask_ptr = digest_cache_verif_get(found_cache, > + "ima"); Instead of using "verif_{set,get}' consider using '{set,get}_usage', where usage here means measure or appraise. > + if (verif_mask_ptr) > + allow_mask = policy_mask & *verif_mask_ptr; > + } > + > + digest_cache_put(digest_cache); > + } > + I'm wondering if it makes sense to create IMA wrappers for each of the digest_cache functions - checking the digest_cache for the hash, setting the digest_cache permitted usage, etc - and put all of them in a separate ima_digest_cache.c file. The file would only be included in the Makefile if digest_cache is configured. In this file you could define a static local global variable to detect whether the digest_cache is ready to be used. Only after successfully measuring and appraising a digest_cache list, based on policy, set the variable. > if (action & IMA_MEASURE) > ima_store_measurement(iint, file, pathname, > xattr_value, xattr_len, modsig, pcr, > - template_desc); > + template_desc, allow_mask); 'allowed_usage'? > if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) { > rc = ima_check_blacklist(iint, modsig, pcr); > if (rc != -EPERM) { thanks, Mimi