Re: [PATCH v5 1/2] ima: add critical data measurement for loaded policy

[Mimi Zohar <zohar@linux.ibm.com>]

On Mon, 2026-07-06 at 23:04 -0400, Mimi Zohar wrote:
> On Thu, 2026-07-02 at 21:04 +0200, Enrico Bravi wrote:
> > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> > index f7f940a76922..a65b7e4b64d6 100644
> > --- a/security/integrity/ima/ima_policy.c
> > +++ b/security/integrity/ima/ima_policy.c
> > @@ -2379,3 +2385,82 @@ bool ima_appraise_signature(enum kernel_read_file_id id)
> >  	return found;
> >  }
> >  #endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
> > +
> > +/**
> > + * ima_measure_loaded_policy - measure the active IMA policy ruleset
> > + *
> > + * Must be called with ima_write_mutex held, as it performs two
> > + * separate RCU read passes over ima_rules and relies on the mutex
> > + * to prevent concurrent policy updates between them.
> > + */
> > +void ima_measure_loaded_policy(void)
> > +{
> > +	const char *event_name = "ima_policy_loaded";
> > +	const char *op = "measure_loaded_ima_policy";
> > +	size_t rule_len = max_rule_len + 2;
> > +	struct ima_rule_entry *rule_entry;
> > +	struct list_head *ima_rules_tmp;
> > +	struct seq_file file;
> 
> Hi Enrico,
> 
> FYI, there's a merge conflict with commit 51bedcd803e0 ("ima: Mediate
> open/release method of the measurements list".
> 
> As long as 2/2 needs to be update, there are two nits: initialize seq_file like
> "file = { 0 };".
> 
> > +	int result = -ENOMEM;
> > +	size_t file_len = 0;
> > +	char *rule;
> > +
> > +	lockdep_assert_held(&ima_write_mutex);
> > +
> > +	rule = vmalloc(rule_len);

FYI, using vmalloc() isn't wrong, but using kmalloc()/kfree() is more common. 
Refer to Documentation/core-api/memory-allocation.rst.

Mimi

> > +	if (!rule) {
> > +		integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, event_name,
> > +				    op, "ENOMEM", result, 0);
> > +		return;
> > +	}
> > +
> > +	/* calculate IMA policy rules memory size */
> > +	file.buf = rule;
> > +	file.read_pos = 0;
> > +	file.size = rule_len;
> > +	file.count = 0;
> > +
> > +	rcu_read_lock();
> > +	ima_rules_tmp = rcu_dereference(ima_rules);
> > +	list_for_each_entry_rcu(rule_entry, ima_rules_tmp, list) {
> > +		ima_policy_show(&file, rule_entry);
> > +
> > +		if (seq_has_overflowed(&file)) {
> > +			result = -E2BIG;
> > +			integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, event_name,
> > +					    op, "rule_length", result, 0);
> 
> and limit the integrity_audit_msg to 80 chars.
> 
> Otherwise the patch looks good.
> 
> Mimi
> 
> > +			rcu_read_unlock();
> > +			goto free_rule;
> > +		}
> > +
> > +		file_len += file.count;
> > +		file.count = 0;
> > +	}
> > +	rcu_read_unlock();
> > +
> > +	/* copy IMA policy rules to a buffer for measuring */
> > +	file.buf = vmalloc(file_len);
> > +	if (!file.buf) {
> > +		integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, event_name,
> > +				    op, "ENOMEM", result, 0);
> > +		goto free_rule;
> > +	}
> > +
> > +	file.read_pos = 0;
> > +	file.size = file_len;
> > +	file.count = 0;
> > +
> > +	rcu_read_lock();
> > +	ima_rules_tmp = rcu_dereference(ima_rules);
> > +	list_for_each_entry_rcu(rule_entry, ima_rules_tmp, list) {
> > +		ima_policy_show(&file, rule_entry);
> > +	}
> > +	rcu_read_unlock();
> > +
> > +	ima_measure_critical_data("ima_policy", event_name, file.buf,
> > +				  file.count, false, NULL, 0);
> > +
> > +	vfree(file.buf);
> > +free_rule:
> > +	vfree(rule);
> > +}