* [PATCH v4 0/2] ima: measure write on securityfs policy file @ 2026-06-17 15:58 Enrico Bravi 2026-06-17 15:58 ` [PATCH v4 1/2] ima: measure loaded policy after " Enrico Bravi 2026-06-17 15:58 ` [PATCH v4 2/2] ima: measure buffer sent to " Enrico Bravi 0 siblings, 2 replies; 11+ messages in thread From: Enrico Bravi @ 2026-06-17 15:58 UTC (permalink / raw) To: linux-integrity, zohar, dmitry.kasatkin, roberto.sassu Cc: eric.snowberg, Enrico Bravi This series aims to introduce integrity measurements when the IMA policy is written on the securityfs file. In particular, when a signed policy is not mandatory, it can be written directly on the securityfs file. This allows to override the boot policy at the first write, and append new policy rules at the subsequent writes (if CONFIG_IMA_WRITE_POLICY=y). In this case new policy can be loaded without being measured. The patch #1 introduces a new critical-data record for the newly loaded policy. The measurement is performed over the textual representation of the new policy once it becomes effective (after ima_update_policy()). As suggested by Mimi, the new critical-data rule is added to the arch specific policy rules (only when a signed policy is not mandatory). The patch #2, following what was suggested by Roberto, measures the input buffer sent to the securityfs policy file, regardless of whether the new policy will be accepted or not. This is done by calling process_buffer_measurement(), enabling POLICY_CHECK in ima_match_rules() and ima_match_rule_data() in order to catch it when 'measure func=POLICY_CHECK' is defined (e.g., ima_policy=tcb). Changes in v4: - Added kernel-doc for the new ima_measure_loaded_policy() function as suggested by Mimi. - Increased the rule buffer size in ima_measure_loaded_policy() (suggested by Mimi) checking if seq_has_overflowed() calculating the policy length. - Acquire ima_write_mutex in before calling ima_measure_loaded_policy() and verify lockdep_assert_held(&ima_write_mutex) as suggested by Mimi. - Initialize file_len to zero in ima_measure_loaded_policy() as suggested by Mimi. - Changed ima_measure_policy_buf() returned error from -ENOPARAM to -EINVAL as suggested by Mimi. - Changed event name from "ima_write_policy_buf" to "ima_policy_written" as suggested by Mimi. - Updated patches description. Changes in v3: - Include the newly defined critical-data rule only if a signed policy is not mandatory as suggested by Mimi. - Removed the ima_policy_text_len() function as suggested by Mimi. - Moved policy input buffer measurement from process_measurement() to process_buffer_measurement() as suggested by Mimi. - Changed ima_measure_policy_write() function name to ima_measure_policy_buf() as suggested by Mimi. - Updated patches description. Changes in v2: - Set a new critical-data rule for measuring the loaded IMA policy. - Add the new critical-data rule to the specific arch policy rules. - Add patch #2 for measuring the input buffer sent to the securityfs policy file. Enrico Bravi (2): ima: measure loaded policy after write on securityfs policy file ima: measure buffer sent to securityfs policy file security/integrity/ima/ima.h | 4 ++ security/integrity/ima/ima_efi.c | 2 + security/integrity/ima/ima_fs.c | 7 ++- security/integrity/ima/ima_main.c | 19 ++++++++ security/integrity/ima/ima_policy.c | 73 ++++++++++++++++++++++++++++- 5 files changed, 102 insertions(+), 3 deletions(-) base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6 -- 2.52.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 1/2] ima: measure loaded policy after write on securityfs policy file 2026-06-17 15:58 [PATCH v4 0/2] ima: measure write on securityfs policy file Enrico Bravi @ 2026-06-17 15:58 ` Enrico Bravi 2026-06-24 20:35 ` Mimi Zohar 2026-06-17 15:58 ` [PATCH v4 2/2] ima: measure buffer sent to " Enrico Bravi 1 sibling, 1 reply; 11+ messages in thread From: Enrico Bravi @ 2026-06-17 15:58 UTC (permalink / raw) To: linux-integrity, zohar, dmitry.kasatkin, roberto.sassu Cc: eric.snowberg, Enrico Bravi 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 <enrico.bravi@polito.it> --- security/integrity/ima/ima.h | 3 ++ security/integrity/ima/ima_efi.c | 2 + security/integrity/ima/ima_fs.c | 6 ++- security/integrity/ima/ima_policy.c | 70 ++++++++++++++++++++++++++++- 4 files changed, 78 insertions(+), 3 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 69e9bf0b82c6..befa221716e5 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -46,6 +46,8 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 }; /* current content of the policy */ extern int ima_policy_flag; +extern struct mutex ima_write_mutex; + /* bitset of digests algorithms allowed in the setxattr hook */ extern atomic_t ima_setxattr_allowed_hash_algorithms; @@ -452,6 +454,7 @@ void *ima_policy_start(struct seq_file *m, loff_t *pos); void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos); void ima_policy_stop(struct seq_file *m, void *v); int ima_policy_show(struct seq_file *m, void *v); +void ima_measure_loaded_policy(void); /* Appraise integrity measurements */ #define IMA_APPRAISE_ENFORCE 0x01 diff --git a/security/integrity/ima/ima_efi.c b/security/integrity/ima/ima_efi.c index bca57d836cb9..be7911009454 100644 --- a/security/integrity/ima/ima_efi.c +++ b/security/integrity/ima/ima_efi.c @@ -17,6 +17,8 @@ static const char * const sb_arch_rules[] = { #endif #if IS_ENABLED(CONFIG_INTEGRITY_MACHINE_KEYRING) && IS_ENABLED(CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY) "appraise func=POLICY_CHECK appraise_type=imasig", +#else + "measure func=CRITICAL_DATA label=ima_policy", #endif "measure func=MODULE_CHECK", NULL diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index ca4931a95098..65e7812c702f 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -24,7 +24,7 @@ #include "ima.h" -static DEFINE_MUTEX(ima_write_mutex); +DEFINE_MUTEX(ima_write_mutex); bool ima_canonical_fmt; static int __init default_canonical_fmt_setup(char *str) @@ -478,6 +478,10 @@ static int ima_release_policy(struct inode *inode, struct file *file) } ima_update_policy(); + + mutex_lock(&ima_write_mutex); + ima_measure_loaded_policy(); + mutex_unlock(&ima_write_mutex); #if !defined(CONFIG_IMA_WRITE_POLICY) && !defined(CONFIG_IMA_READ_POLICY) securityfs_remove(file->f_path.dentry); #elif defined(CONFIG_IMA_WRITE_POLICY) 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 @@ -17,6 +17,7 @@ #include <linux/slab.h> #include <linux/rculist.h> #include <linux/seq_file.h> +#include <linux/vmalloc.h> #include <linux/ima.h> #include "ima.h" @@ -2020,7 +2021,6 @@ const char *const func_tokens[] = { __ima_hooks(__ima_hook_stringify) }; -#ifdef CONFIG_IMA_READ_POLICY enum { mask_exec = 0, mask_write, mask_read, mask_append }; @@ -2322,7 +2322,6 @@ int ima_policy_show(struct seq_file *m, void *v) seq_puts(m, "\n"); return 0; } -#endif /* CONFIG_IMA_READ_POLICY */ #if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING) /* @@ -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; + } + + 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); +} -- 2.52.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/2] ima: measure loaded policy after write on securityfs policy file 2026-06-17 15:58 ` [PATCH v4 1/2] ima: measure loaded policy after " Enrico Bravi @ 2026-06-24 20:35 ` Mimi Zohar 2026-06-25 14:17 ` Mimi Zohar 2026-06-26 9:32 ` Enrico Bravi 0 siblings, 2 replies; 11+ messages in thread From: Mimi Zohar @ 2026-06-24 20:35 UTC (permalink / raw) To: Enrico Bravi, linux-integrity, dmitry.kasatkin, roberto.sassu Cc: eric.snowberg 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 <enrico.bravi@polito.it> 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); > +} ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/2] ima: measure loaded policy after write on securityfs policy file 2026-06-24 20:35 ` Mimi Zohar @ 2026-06-25 14:17 ` Mimi Zohar 2026-06-26 9:36 ` Enrico Bravi 2026-06-26 9:32 ` Enrico Bravi 1 sibling, 1 reply; 11+ messages in thread From: Mimi Zohar @ 2026-06-25 14:17 UTC (permalink / raw) To: Enrico Bravi, linux-integrity, dmitry.kasatkin, roberto.sassu Cc: eric.snowberg On Wed, 2026-06-24 at 16:35 -0400, Mimi Zohar wrote: [...] > > 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. > > +*/ Hi Enrico, I forgot to mention that the kernel-doc formatting is off. Refer to checkpatch.pl for the details. Either change it to a regular comment or format it properly. thanks, Mimi > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/2] ima: measure loaded policy after write on securityfs policy file 2026-06-25 14:17 ` Mimi Zohar @ 2026-06-26 9:36 ` Enrico Bravi 0 siblings, 0 replies; 11+ messages in thread From: Enrico Bravi @ 2026-06-26 9:36 UTC (permalink / raw) To: roberto.sassu@huawei.com, linux-integrity@vger.kernel.org, zohar@linux.ibm.com, dmitry.kasatkin@gmail.com Cc: eric.snowberg@oracle.com On Thu, 2026-06-25 at 10:17 -0400, Mimi Zohar wrote: > On Wed, 2026-06-24 at 16:35 -0400, Mimi Zohar wrote: > [...] > > > > 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. > > > +*/ > > Hi Enrico, > > I forgot to mention that the kernel-doc formatting is off. Refer to > checkpatch.pl for the details. Either change it to a regular comment or > format > it properly. Hi Mimi, yes, I forgot to check it, I'm sorry about that. I'll fix it. Thank you, Enrico > thanks, > > Mimi > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/2] ima: measure loaded policy after write on securityfs policy file 2026-06-24 20:35 ` Mimi Zohar 2026-06-25 14:17 ` Mimi Zohar @ 2026-06-26 9:32 ` Enrico Bravi 2026-06-26 13:37 ` Mimi Zohar 1 sibling, 1 reply; 11+ messages in thread From: Enrico Bravi @ 2026-06-26 9:32 UTC (permalink / raw) To: roberto.sassu@huawei.com, linux-integrity@vger.kernel.org, zohar@linux.ibm.com, dmitry.kasatkin@gmail.com Cc: eric.snowberg@oracle.com 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 <enrico.bravi@polito.it> > > 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); > > +} ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/2] ima: measure loaded policy after write on securityfs policy file 2026-06-26 9:32 ` Enrico Bravi @ 2026-06-26 13:37 ` Mimi Zohar 0 siblings, 0 replies; 11+ messages in thread From: Mimi Zohar @ 2026-06-26 13:37 UTC (permalink / raw) To: Enrico Bravi, roberto.sassu@huawei.com, linux-integrity@vger.kernel.org, dmitry.kasatkin@gmail.com Cc: eric.snowberg@oracle.com On Fri, 2026-06-26 at 09:32 +0000, Enrico Bravi wrote: > > > > > 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. Right, it isn't necessary, but there's no harm in keeping it either. > > > Missing rcu_read_unlock() before returning. > > thanks, Mimi ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 2/2] ima: measure buffer sent to securityfs policy file 2026-06-17 15:58 [PATCH v4 0/2] ima: measure write on securityfs policy file Enrico Bravi 2026-06-17 15:58 ` [PATCH v4 1/2] ima: measure loaded policy after " Enrico Bravi @ 2026-06-17 15:58 ` Enrico Bravi 2026-06-25 1:05 ` Mimi Zohar 1 sibling, 1 reply; 11+ messages in thread From: Enrico Bravi @ 2026-06-17 15:58 UTC (permalink / raw) To: linux-integrity, zohar, dmitry.kasatkin, roberto.sassu Cc: eric.snowberg, Enrico Bravi When a signed policy is not mandatory, it is possible to write the IMA policy directly on the corresponding securityfs file: echo -e "measure func=BPRM_CHECK mask=MAY_EXEC\n" \ "audit func=BPRM_CHECK mask=MAY_EXEC\n" \ > /sys/kernel/security/ima/policy or by cat'ing the entire IMA custom policy file: cat ima-policy-file > /sys/kernel/security/ima/policy Add input buffer measurement, regardless of whether the new policy will be accepted or not, that can be caught when 'measure func=POLICY_CHECK' is enabled (e.g., ima_policy=tcb). The measurement template is forced to ima-buf. This follows the "measure & load" paradigm, exposing potential bugs in the policy code and detecting attempts to corrupt IMA. It also completes the POLICY_CHECK hook, which already measures partial policy load by file. To verify the template data hash value, convert the buffer policy data to binary: grep "ima_policy_written" \ /sys/kernel/security/integrity/ima/ascii_runtime_measurements | \ tail -1 | cut -d' ' -f 6 | xxd -r -p | sha256sum Suggested-by: Roberto Sassu <roberto.sassu@huawei.com> Signed-off-by: Enrico Bravi <enrico.bravi@polito.it> --- security/integrity/ima/ima.h | 1 + security/integrity/ima/ima_fs.c | 1 + security/integrity/ima/ima_main.c | 19 +++++++++++++++++++ security/integrity/ima/ima_policy.c | 3 +++ 4 files changed, 24 insertions(+) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index befa221716e5..d477fc06821f 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -455,6 +455,7 @@ void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos); void ima_policy_stop(struct seq_file *m, void *v); int ima_policy_show(struct seq_file *m, void *v); void ima_measure_loaded_policy(void); +int ima_measure_policy_buf(const char *buf, size_t buf_len); /* Appraise integrity measurements */ #define IMA_APPRAISE_ENFORCE 0x01 diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 65e7812c702f..a277c9135944 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -356,6 +356,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, 1, 0); result = -EACCES; } else { + ima_measure_policy_buf(data, datalen); result = ima_parse_add_rule(data); } mutex_unlock(&ima_write_mutex); diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 5cea53fc36df..599495304712 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -1221,6 +1221,25 @@ int ima_measure_critical_data(const char *event_label, } EXPORT_SYMBOL_GPL(ima_measure_critical_data); +/** + * ima_measure_policy_buf - Measure the policy write buffer + * @buf: pointer to the buffer containing the policy write data + * @buf_len: size of the buffer + * + * Measure the buffer sent to the IMA policy securityfs file. + * + * Return 0 on success, a negative value otherwise. + */ +int ima_measure_policy_buf(const char *buf, size_t buf_len) +{ + if (!buf || !buf_len) + return -EINVAL; + + return process_buffer_measurement(&nop_mnt_idmap, NULL, buf, buf_len, + "ima_policy_written", POLICY_CHECK, 0, + NULL, false, NULL, 0); +} + #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS /** diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 0a70d10da70a..fc747f391e41 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -541,6 +541,8 @@ static bool ima_match_rule_data(struct ima_rule_entry *rule, opt_list = rule->label; break; + case POLICY_CHECK: + return true; default: return false; } @@ -589,6 +591,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, switch (func) { case KEY_CHECK: case CRITICAL_DATA: + case POLICY_CHECK: return ((rule->func == func) && ima_match_rule_data(rule, func_data, cred)); default: -- 2.52.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] ima: measure buffer sent to securityfs policy file 2026-06-17 15:58 ` [PATCH v4 2/2] ima: measure buffer sent to " Enrico Bravi @ 2026-06-25 1:05 ` Mimi Zohar 2026-06-29 9:26 ` Enrico Bravi 0 siblings, 1 reply; 11+ messages in thread From: Mimi Zohar @ 2026-06-25 1:05 UTC (permalink / raw) To: Enrico Bravi, linux-integrity, dmitry.kasatkin, roberto.sassu Cc: eric.snowberg The Subject line describes "how", not "what". Consider renaming it to "ima: measure userspace policy writes before parsing". On Wed, 2026-06-17 at 17:58 +0200, Enrico Bravi wrote: > When a signed policy is not mandatory, it is possible to write the IMA > policy directly on the corresponding securityfs file: When a signed policy is not mandatory, userspace can write IMA policy rules directly to the securityfs policy file: > > echo -e "measure func=BPRM_CHECK mask=MAY_EXEC\n" \ > "audit func=BPRM_CHECK mask=MAY_EXEC\n" \ > > /sys/kernel/security/ima/policy > > or by cat'ing the entire IMA custom policy file: > > cat ima-policy-file > /sys/kernel/security/ima/policy Because these rules originate from userspace and cross the userspace/kernel trust boundary, measure the raw write buffer before parsing. > > Add input buffer measurement, regardless of whether the new policy > will be accepted or not, that can be caught when > 'measure func=POLICY_CHECK' is enabled (e.g., ima_policy=tcb). The > measurement template is forced to ima-buf. > This follows the "measure & load" paradigm, exposing potential bugs in > the policy code and detecting attempts to corrupt IMA. It also completes > the POLICY_CHECK hook, which already measures partial policy load by file. > > To verify the template data hash value, convert the buffer policy data > to binary: > grep "ima_policy_written" \ > /sys/kernel/security/integrity/ima/ascii_runtime_measurements | \ > tail -1 | cut -d' ' -f 6 | xxd -r -p | sha256sum > > Suggested-by: Roberto Sassu <roberto.sassu@huawei.com> > Signed-off-by: Enrico Bravi <enrico.bravi@polito.it> > --- > security/integrity/ima/ima.h | 1 + > security/integrity/ima/ima_fs.c | 1 + > security/integrity/ima/ima_main.c | 19 +++++++++++++++++++ > security/integrity/ima/ima_policy.c | 3 +++ > 4 files changed, 24 insertions(+) > > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > index befa221716e5..d477fc06821f 100644 > --- a/security/integrity/ima/ima.h > +++ b/security/integrity/ima/ima.h > @@ -455,6 +455,7 @@ void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos); > void ima_policy_stop(struct seq_file *m, void *v); > int ima_policy_show(struct seq_file *m, void *v); > void ima_measure_loaded_policy(void); > +int ima_measure_policy_buf(const char *buf, size_t buf_len); > > /* Appraise integrity measurements */ > #define IMA_APPRAISE_ENFORCE 0x01 > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c > index 65e7812c702f..a277c9135944 100644 > --- a/security/integrity/ima/ima_fs.c > +++ b/security/integrity/ima/ima_fs.c > @@ -356,6 +356,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, > 1, 0); > result = -EACCES; > } else { > + ima_measure_policy_buf(data, datalen); Should failure to measure the input policy rules be audited? > result = ima_parse_add_rule(data); > } > mutex_unlock(&ima_write_mutex); > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c > index 5cea53fc36df..599495304712 100644 > --- a/security/integrity/ima/ima_main.c > +++ b/security/integrity/ima/ima_main.c > @@ -1221,6 +1221,25 @@ int ima_measure_critical_data(const char *event_label, > } > EXPORT_SYMBOL_GPL(ima_measure_critical_data); > > +/** > + * ima_measure_policy_buf - Measure the policy write buffer Consider renaming this function to ima_measure_policy_input(), which parallels the function ima_measure_loaded_policy() in the first patch. Mimi > + * @buf: pointer to the buffer containing the policy write data > + * @buf_len: size of the buffer > + * > + * Measure the buffer sent to the IMA policy securityfs file. > + * > + * Return 0 on success, a negative value otherwise. > + */ > +int ima_measure_policy_buf(const char *buf, size_t buf_len) > +{ > + if (!buf || !buf_len) > + return -EINVAL; > + > + return process_buffer_measurement(&nop_mnt_idmap, NULL, buf, buf_len, > + "ima_policy_written", POLICY_CHECK, 0, > + NULL, false, NULL, 0); > +} > + > #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS > > /** ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] ima: measure buffer sent to securityfs policy file 2026-06-25 1:05 ` Mimi Zohar @ 2026-06-29 9:26 ` Enrico Bravi 2026-06-29 16:38 ` Mimi Zohar 0 siblings, 1 reply; 11+ messages in thread From: Enrico Bravi @ 2026-06-29 9:26 UTC (permalink / raw) To: roberto.sassu@huawei.com, linux-integrity@vger.kernel.org, zohar@linux.ibm.com, dmitry.kasatkin@gmail.com Cc: eric.snowberg@oracle.com Hi Mimi, On Wed, 2026-06-24 at 21:05 -0400, Mimi Zohar wrote: > The Subject line describes "how", not "what". Consider renaming it to "ima: > measure userspace policy writes before parsing". thank you, will fix it. > On Wed, 2026-06-17 at 17:58 +0200, Enrico Bravi wrote: > > When a signed policy is not mandatory, it is possible to write the IMA > > policy directly on the corresponding securityfs file: > > When a signed policy is not mandatory, userspace can write IMA policy rules > directly to the securityfs policy file: > > > > echo -e "measure func=BPRM_CHECK mask=MAY_EXEC\n" \ > > "audit func=BPRM_CHECK mask=MAY_EXEC\n" \ > > > /sys/kernel/security/ima/policy > > > > or by cat'ing the entire IMA custom policy file: > > > > cat ima-policy-file > /sys/kernel/security/ima/policy > > Because these rules originate from userspace and cross the userspace/kernel > trust boundary, measure the raw write buffer before parsing. Thanks for these suggestions, I'll update the patch description. > > > > Add input buffer measurement, regardless of whether the new policy > > will be accepted or not, that can be caught when > > 'measure func=POLICY_CHECK' is enabled (e.g., ima_policy=tcb). The > > measurement template is forced to ima-buf. > > > This follows the "measure & load" paradigm, exposing potential bugs in > > the policy code and detecting attempts to corrupt IMA. It also completes > > the POLICY_CHECK hook, which already measures partial policy load by file. > > > > > To verify the template data hash value, convert the buffer policy data > > to binary: > > grep "ima_policy_written" \ > > /sys/kernel/security/integrity/ima/ascii_runtime_measurements | \ > > tail -1 | cut -d' ' -f 6 | xxd -r -p | sha256sum > > > > Suggested-by: Roberto Sassu <roberto.sassu@huawei.com> > > Signed-off-by: Enrico Bravi <enrico.bravi@polito.it> > > --- > > security/integrity/ima/ima.h | 1 + > > security/integrity/ima/ima_fs.c | 1 + > > security/integrity/ima/ima_main.c | 19 +++++++++++++++++++ > > security/integrity/ima/ima_policy.c | 3 +++ > > 4 files changed, 24 insertions(+) > > > > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > > index befa221716e5..d477fc06821f 100644 > > --- a/security/integrity/ima/ima.h > > +++ b/security/integrity/ima/ima.h > > @@ -455,6 +455,7 @@ void *ima_policy_next(struct seq_file *m, void *v, > > loff_t *pos); > > void ima_policy_stop(struct seq_file *m, void *v); > > int ima_policy_show(struct seq_file *m, void *v); > > void ima_measure_loaded_policy(void); > > +int ima_measure_policy_buf(const char *buf, size_t buf_len); > > > > /* Appraise integrity measurements */ > > #define IMA_APPRAISE_ENFORCE 0x01 > > diff --git a/security/integrity/ima/ima_fs.c > > b/security/integrity/ima/ima_fs.c > > index 65e7812c702f..a277c9135944 100644 > > --- a/security/integrity/ima/ima_fs.c > > +++ b/security/integrity/ima/ima_fs.c > > @@ -356,6 +356,7 @@ static ssize_t ima_write_policy(struct file *file, const > > char __user *buf, > > 1, 0); > > result = -EACCES; > > } else { > > + ima_measure_policy_buf(data, datalen); > > Should failure to measure the input policy rules be audited? process_buffer_measurement() is already auditing in case of failure before returning. Do you think it is necessary to audit also at this point? > > result = ima_parse_add_rule(data); > > } > > mutex_unlock(&ima_write_mutex); > > diff --git a/security/integrity/ima/ima_main.c > > b/security/integrity/ima/ima_main.c > > index 5cea53fc36df..599495304712 100644 > > --- a/security/integrity/ima/ima_main.c > > +++ b/security/integrity/ima/ima_main.c > > @@ -1221,6 +1221,25 @@ int ima_measure_critical_data(const char > > *event_label, > > } > > EXPORT_SYMBOL_GPL(ima_measure_critical_data); > > > > +/** > > + * ima_measure_policy_buf - Measure the policy write buffer > > Consider renaming this function to ima_measure_policy_input(), which parallels > the function ima_measure_loaded_policy() in the first patch. My intention with the previous ima_measure_policy_write() name was to highlight the fact it is not measuring every data sent to the policy file. For example, writing the path of the file from which reading the new policy does not trigger this measurement. Eventually, what do you think of ima_measure_raw_policy() or ima_measure_unparsed_policy()? Thank you very much, Enrico > Mimi > > > + * @buf: pointer to the buffer containing the policy write data > > + * @buf_len: size of the buffer > > + * > > + * Measure the buffer sent to the IMA policy securityfs file. > > + * > > + * Return 0 on success, a negative value otherwise. > > + */ > > +int ima_measure_policy_buf(const char *buf, size_t buf_len) > > +{ > > + if (!buf || !buf_len) > > + return -EINVAL; > > + > > + return process_buffer_measurement(&nop_mnt_idmap, NULL, buf, > > buf_len, > > + "ima_policy_written", > > POLICY_CHECK, 0, > > + NULL, false, NULL, 0); > > +} > > + > > #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS > > > > /** ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 2/2] ima: measure buffer sent to securityfs policy file 2026-06-29 9:26 ` Enrico Bravi @ 2026-06-29 16:38 ` Mimi Zohar 0 siblings, 0 replies; 11+ messages in thread From: Mimi Zohar @ 2026-06-29 16:38 UTC (permalink / raw) To: Enrico Bravi, roberto.sassu@huawei.com, linux-integrity@vger.kernel.org, dmitry.kasatkin@gmail.com Cc: eric.snowberg@oracle.com On Mon, 2026-06-29 at 09:26 +0000, Enrico Bravi wrote: > > > diff --git a/security/integrity/ima/ima_fs.c > > > b/security/integrity/ima/ima_fs.c > > > index 65e7812c702f..a277c9135944 100644 > > > --- a/security/integrity/ima/ima_fs.c > > > +++ b/security/integrity/ima/ima_fs.c > > > @@ -356,6 +356,7 @@ static ssize_t ima_write_policy(struct file *file, const > > > char __user *buf, > > > 1, 0); > > > result = -EACCES; > > > } else { > > > + ima_measure_policy_buf(data, datalen); > > > > Should failure to measure the input policy rules be audited? > > process_buffer_measurement() is already auditing in case of failure before > returning. Do you think it is necessary to audit also at this point? No, you're correct. > > > > result = ima_parse_add_rule(data); > > > } > > > mutex_unlock(&ima_write_mutex); > > > diff --git a/security/integrity/ima/ima_main.c > > > b/security/integrity/ima/ima_main.c > > > index 5cea53fc36df..599495304712 100644 > > > --- a/security/integrity/ima/ima_main.c > > > +++ b/security/integrity/ima/ima_main.c > > > @@ -1221,6 +1221,25 @@ int ima_measure_critical_data(const char > > > *event_label, > > > } > > > EXPORT_SYMBOL_GPL(ima_measure_critical_data); > > > > > > +/** > > > + * ima_measure_policy_buf - Measure the policy write buffer > > > > Consider renaming this function to ima_measure_policy_input(), which parallels > > the function ima_measure_loaded_policy() in the first patch. > > My intention with the previous ima_measure_policy_write() name was to highlight > the fact it is not measuring every data sent to the policy file. For example, > writing the path of the file from which reading the new policy does not trigger > this measurement. Eventually, what do you think of ima_measure_raw_policy() or > ima_measure_unparsed_policy()? The policy could have comment lines, which are being measured. Naming the function ima_measure_raw_policy() is perfect. > thanks, Mimi ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-06-29 16:44 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-17 15:58 [PATCH v4 0/2] ima: measure write on securityfs policy file Enrico Bravi 2026-06-17 15:58 ` [PATCH v4 1/2] ima: measure loaded policy after " Enrico Bravi 2026-06-24 20:35 ` Mimi Zohar 2026-06-25 14:17 ` Mimi Zohar 2026-06-26 9:36 ` Enrico Bravi 2026-06-26 9:32 ` Enrico Bravi 2026-06-26 13:37 ` Mimi Zohar 2026-06-17 15:58 ` [PATCH v4 2/2] ima: measure buffer sent to " Enrico Bravi 2026-06-25 1:05 ` Mimi Zohar 2026-06-29 9:26 ` Enrico Bravi 2026-06-29 16:38 ` Mimi Zohar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox