[PATCH v5 0/2] ima: measure write on securityfs policy file

[PATCH v5 0/2] ima: measure write on securityfs policy file

[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 v5:
 - Changed patches' subject as suggested by Mimi.
 - Added max_rule_len variable for tracking the longest serialized rule
   as suggested by Mimi.
 - Updated patches description.
 - Changed ima_measure_policy_buf() name to ima_measure_raw_policy() and
   updated function description as suggested by Mimi.

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: add critical data measurement for loaded policy
  ima: measure userspace policy writes before parsing

 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   | 22 +++++++
 security/integrity/ima/ima_policy.c | 92 ++++++++++++++++++++++++++++-
 5 files changed, 124 insertions(+), 3 deletions(-)


base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
-- 
2.52.0

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

[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 | 89 ++++++++++++++++++++++++++++-
 4 files changed, 97 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..a65b7e4b64d6 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"
@@ -53,6 +54,8 @@
 #define INVALID_PCR(a) (((a) < 0) || \
 	(a) >= (sizeof_field(struct ima_iint_cache, measured_pcrs) * 8))
 
+static size_t max_rule_len;
+
 int ima_policy_flag;
 static int temp_ima_appraise;
 static int build_ima_appraise __ro_after_init;
@@ -955,6 +958,8 @@ void __init ima_init_policy(void)
 {
 	int build_appraise_entries, arch_entries;
 
+	max_rule_len = 255;
+
 	/* if !ima_policy, we load NO default rules */
 	if (ima_policy)
 		add_rules(dont_measure_rules, ARRAY_SIZE(dont_measure_rules),
@@ -1993,6 +1998,9 @@ ssize_t ima_parse_add_rule(char *rule)
 
 	list_add_tail(&entry->list, &ima_temp_rules);
 
+	if (len > max_rule_len)
+		max_rule_len = len;
+
 	return len;
 }
 
@@ -2020,7 +2028,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 +2329,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 +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;
+	int result = -ENOMEM;
+	size_t file_len = 0;
+	char *rule;
+
+	lockdep_assert_held(&ima_write_mutex);
+
+	rule = vmalloc(rule_len);
+	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);
+			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);
+}
-- 
2.52.0

[PATCH v5 2/2] ima: measure userspace policy writes before parsing

[Enrico Bravi]

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, regardless of
whether the new policy will be accepted or not. This 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   | 22 ++++++++++++++++++++++
 security/integrity/ima/ima_policy.c |  3 +++
 4 files changed, 27 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index befa221716e5..8d60dcf9c689 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_raw_policy(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..fed97eb625dd 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_raw_policy(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..5277e1f680f0 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -1221,6 +1221,28 @@ int ima_measure_critical_data(const char *event_label,
 }
 EXPORT_SYMBOL_GPL(ima_measure_critical_data);
 
+/**
+ * ima_measure_raw_policy - Measure the raw policy write buffer
+ * @buf: pointer to the buffer containing the raw policy data
+ * @buf_len: size of the buffer
+ *
+ * Measure the raw policy buffer sent to the IMA policy securityfs file. The
+ * buffer is written from userspace, and the measurement is performed before
+ * parsing it. This measurement includes any data written on the policy file
+ * such as malformed policy rules and comments.
+ *
+ * Return 0 on success, a negative value otherwise.
+ */
+int ima_measure_raw_policy(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 a65b7e4b64d6..d6d249190705 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -543,6 +543,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;
 	}
@@ -591,6 +593,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

Re: [PATCH v5 2/2] ima: measure userspace policy writes before parsing

[Mimi Zohar]

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 a65b7e4b64d6..d6d249190705 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -591,6 +593,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:

Hi Enrico,

Unlike the other hooks, KEY_CHECK and CRITICAL_DATA are special cases, which
only allow a set of keyrings or labels respectively.  POLICY_CHECK rules can be
defined in terms of other file metadata (e.g. uid, gid, ...). With this change,
any options specified on the rule will be not be matched.

ima_match_rule_data() should only be called for buffer measurements, when there
is no inode. 

+               return ((rule->func == func) && !inode &&
                        ima_match_rule_data(rule, func_data, cred));

Otherwise the patch looks good.

Mimi

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

[Mimi Zohar]

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

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

[Enrico Bravi]

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".

Hi Mimi,

yes, I was still basing it on commit 8cd9520d35a6 ("Linux 7.1"). I'll rebase it
and fix what you suggested.

> 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);
> > +	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.

Thank you very much.

Enrico

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

Re: [PATCH v5 2/2] ima: measure userspace policy writes before parsing

[Enrico Bravi]

On Mon, 2026-07-06 at 23:03 -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 a65b7e4b64d6..d6d249190705 100644
> > --- a/security/integrity/ima/ima_policy.c
> > +++ b/security/integrity/ima/ima_policy.c
> > @@ -591,6 +593,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:
> 
> Hi Enrico,
> 
> Unlike the other hooks, KEY_CHECK and CRITICAL_DATA are special cases, which
> only allow a set of keyrings or labels respectively.  POLICY_CHECK rules can
> be
> defined in terms of other file metadata (e.g. uid, gid, ...). With this
> change,
> any options specified on the rule will be not be matched.

Hi Mimi,

you're right.

> ima_match_rule_data() should only be called for buffer measurements, when
> there
> is no inode. 
> 
> +               return ((rule->func == func) && !inode &&
>                         ima_match_rule_data(rule, func_data, cred));

I was thinking that in this way, it would not trigger the measurement when
loading the policy from a file. If inode is not NULL, it directly returns false
instead of continuing. What do you thing of something like this:

        switch (func) {
+       case POLICY_CHECK:
+               if (inode)
+                       break;
+               fallthrough;
        case KEY_CHECK:
        case CRITICAL_DATA:
                return ((rule->func == func) &&
                        ima_match_rule_data(rule, func_data, cred));

> Otherwise the patch looks good.

Thank you,

Enrico

> Mimi

Re: [PATCH v5 2/2] ima: measure userspace policy writes before parsing

[Mimi Zohar]

On Tue, 2026-07-07 at 15:25 +0000, Enrico  Bravi wrote:
> On Mon, 2026-07-06 at 23:03 -0400, Mimi Zohar wrote


> > ima_match_rule_data() should only be called for buffer measurements, when
> > there
> > is no inode. 
> > 
> > +               return ((rule->func == func) && !inode &&
> >                         ima_match_rule_data(rule, func_data, cred));
> 
> I was thinking that in this way, it would not trigger the measurement when
> loading the policy from a file. If inode is not NULL, it directly returns false
> instead of continuing. What do you thing of something like this:
> 
>         switch (func) {
> +       case POLICY_CHECK:
> +               if (inode)
> +                       break;
> +               fallthrough;
>         case KEY_CHECK:
>         case CRITICAL_DATA:
>                 return ((rule->func == func) &&
>                         ima_match_rule_data(rule, func_data, cred));

Yes, your version is clearer.

Mimi

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

[Mimi Zohar]

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