Hi Mimi, On Wed, 2026-06-24 at 16:35 -0400, Mimi Zohar wrote: > 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".
yes, will change it. > 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. Yes, this is much better. In this way the check on seq_has_overflowed() should not be necessary anymore. Thank you very much for your suggestions. Enrico > 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); > > +}
