Re: [PATCH v4 1/2] ima: measure loaded policy after write on securityfs policy file

The Subject line needs to be written from a higher perspective.  It describes
"how", not "what".
Consider using "ima: add critical data measurement for loaded policy".

On Wed, 2026-06-17 at 17:58 +0200, Enrico Bravi wrote:
> IMA policy can be written multiple times in the securityfs policy file
> at runtime if CONFIG_IMA_WRITE_POLICY=y. When IMA_APPRAISE_POLICY is
> required, the policy needs to be signed to be loaded, writing the absolute
> path of the file containing the new policy:
> 
> echo /path/of/custom_ima_policy > /sys/kernel/security/ima/policy
> 
> When this is not required, policy can be written directly, rule by rule:
> 
> echo -e "measure func=BPRM_CHECK mask=MAY_EXEC\n" \
>         "audit func=BPRM_CHECK mask=MAY_EXEC\n" \
>      > /sys/kernel/security/ima/policy
> 
> In this case, a new policy can be loaded without being measured or
> appraised.
> 
> Add a new critical data record to measure the textual policy
> representation when it becomes effective. Include in the
> architecture-specific policy the new critical data record only when it
> is not mandatory to load a signed policy. Additionally, enable the
> policy serialization code even when CONFIG_IMA_READ_POLICY=n.
> 
> To verify the template data hash value, convert the buffer policy data
> to binary:
> grep "ima_policy_loaded" \
>         /sys/kernel/security/integrity/ima/ascii_runtime_measurements | \
>         tail -1 | cut -d' ' -f 6 | xxd -r -p | sha256sum
> 
> Signed-off-by: Enrico Bravi <[email protected]>

Thank you for making the changes.

[ ... ]

> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index f7f940a76922..0a70d10da70a 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c

> @@ -2379,3 +2378,70 @@ 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";
> +	struct ima_rule_entry *rule_entry;
> +	struct list_head *ima_rules_tmp;
> +	struct seq_file file;
> +	int result = -ENOMEM;
> +	size_t file_len = 0;
> +	char rule[512];
> +
> +	/* calculate IMA policy rules memory size */
> +	file.buf = rule;
> +	file.read_pos = 0;
> +	file.size = 512;
> +	file.count = 0;
> +
> +	lockdep_assert_held(&ima_write_mutex);
> +
> +	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, 1);
> +			return;

On failure the new IMA policy will not be measured. Instead of hard coding the
buffer to 512, define a file static global variable to keep track of the maximum
policy rule size.  ima_parse_add_rule() already returns the policy rule length.
Before returning update the max policy rule size variable as necessary.

Here in ima_measure_loaded_policy() allocate/free the buffer.

Missing rcu_read_unlock() before returning.

thanks,

Mimi

> +		}
> +
> +		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, 1);
> +		return;
> +	}
> +
> +	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);
> +}