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

[RFC][PATCH v3 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 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        |  2 +
 security/integrity/ima/ima_efi.c    |  2 +
 security/integrity/ima/ima_fs.c     |  2 +
 security/integrity/ima/ima_main.c   | 19 ++++++++++
 security/integrity/ima/ima_policy.c | 58 ++++++++++++++++++++++++++++-
 5 files changed, 81 insertions(+), 2 deletions(-)


base-commit: 028ef9c96e96197026887c0f092424679298aae8
-- 
2.52.0

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

[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.
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        |  1 +
 security/integrity/ima/ima_efi.c    |  2 ++
 security/integrity/ima/ima_fs.c     |  1 +
 security/integrity/ima/ima_policy.c | 55 +++++++++++++++++++++++++++--
 4 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 89ebe98ffc5e..a223d3f30d88 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -425,6 +425,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 138029bfcce1..8e9f85ec9a86 100644
--- a/security/integrity/ima/ima_efi.c
+++ b/security/integrity/ima/ima_efi.c
@@ -60,6 +60,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 012a58959ff0..75cb308cf01f 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -476,6 +476,7 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 	}
 
 	ima_update_policy();
+	ima_measure_loaded_policy();
 #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 bf2d7ba4c14a..e0b4dae922b6 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"
@@ -2022,7 +2023,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
 };
@@ -2324,7 +2324,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)
 /*
@@ -2381,3 +2380,55 @@ bool ima_appraise_signature(enum kernel_read_file_id id)
 	return found;
 }
 #endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
+
+void ima_measure_loaded_policy(void)
+{
+	const char *event_name = "ima_policy_loaded";
+	const char *op = "measure_loaded_ima_policy";
+	const char *audit_cause = "ENOMEM";
+	struct ima_rule_entry *rule_entry;
+	struct list_head *ima_rules_tmp;
+	struct seq_file file;
+	int result = -ENOMEM;
+	size_t file_len;
+	char rule[255];
+
+	/* calculate IMA policy rules memory size */
+	file.buf = rule;
+	file.read_pos = 0;
+	file.size = 255;
+	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);
+		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, audit_cause, 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

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

[Mimi Zohar]

On Tue, 2026-05-26 at 15:51 +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.
> 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>

Thanks, Enrico.  Just a few inline comments.

> ---
>  security/integrity/ima/ima.h        |  1 +
>  security/integrity/ima/ima_efi.c    |  2 ++
>  security/integrity/ima/ima_fs.c     |  1 +
>  security/integrity/ima/ima_policy.c | 55 +++++++++++++++++++++++++++--
>  4 files changed, 57 insertions(+), 2 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 89ebe98ffc5e..a223d3f30d88 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -425,6 +425,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 138029bfcce1..8e9f85ec9a86 100644
> --- a/security/integrity/ima/ima_efi.c
> +++ b/security/integrity/ima/ima_efi.c
> @@ -60,6 +60,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

 None of the other arch "measure" policy rules are conditional.  Should the new
"measure" rule be limited?

>  	"measure func=MODULE_CHECK",
>  	NULL
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 012a58959ff0..75cb308cf01f 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -476,6 +476,7 @@ static int ima_release_policy(struct inode *inode, struct file *file)
>  	}
>  
>  	ima_update_policy();
> +	ima_measure_loaded_policy();
>  #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 bf2d7ba4c14a..e0b4dae922b6 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"
> @@ -2022,7 +2023,6 @@ const char *const func_tokens[] = {
>  	__ima_hooks(__ima_hook_stringify)
>  };
>  
> -#ifdef	CONFIG_IMA_READ_POLICY

Removing the ifdef, here, does not affect viewing the IMA measurement lists, but
allows copying and measuring the policy rules.  Please include a comment in the
patch description.

>  enum {
>  	mask_exec = 0, mask_write, mask_read, mask_append
>  };
> @@ -2324,7 +2324,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)
>  /*
> @@ -2381,3 +2380,55 @@ bool ima_appraise_signature(enum kernel_read_file_id id)
>  	return found;
>  }
>  #endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
> +

Please add kernel-doc here, something like:

/**
 * 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";
> +	const char *audit_cause = "ENOMEM";
> +	struct ima_rule_entry *rule_entry;
> +	struct list_head *ima_rules_tmp;
> +	struct seq_file file;
> +	int result = -ENOMEM;
> +	size_t file_len;
> +	char rule[255];

The 255-byte buffer may be insufficient for custom policy rules that include
additional fields such as LSM labels and other file metadata, unlike the simpler
built-in and architecture-specific rules. Please increase the buffer size to
accommodate the worst-case serialized rule length.

> +
> +	/* calculate IMA policy rules memory size */
> +	file.buf = rule;
> +	file.read_pos = 0;
> +	file.size = 255;
> +	file.count = 0;
> +

Please add "lockdep_assert_held(&ima_write_mutex);"  here.

> +	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);
> +		file_len += file.count;
> +		file.count = 0;
> +	}

Variables defined on the stack need to be initialized before being used. Please
iniitalize file_len to zero.

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

Thanks,

Mimi

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

[Enrico Bravi]

Hi Mimi,

On Wed, 2026-06-10 at 10:32 -0400, Mimi Zohar wrote:
> On Tue, 2026-05-26 at 15:51 +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.
> > 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>
> 
> Thanks, Enrico.  Just a few inline comments.

Thank you very much for your feedback.

> > ---
> >  security/integrity/ima/ima.h        |  1 +
> >  security/integrity/ima/ima_efi.c    |  2 ++
> >  security/integrity/ima/ima_fs.c     |  1 +
> >  security/integrity/ima/ima_policy.c | 55 +++++++++++++++++++++++++++--
> >  4 files changed, 57 insertions(+), 2 deletions(-)
> > 
> > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> > index 89ebe98ffc5e..a223d3f30d88 100644
> > --- a/security/integrity/ima/ima.h
> > +++ b/security/integrity/ima/ima.h
> > @@ -425,6 +425,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 138029bfcce1..8e9f85ec9a86 100644
> > --- a/security/integrity/ima/ima_efi.c
> > +++ b/security/integrity/ima/ima_efi.c
> > @@ -60,6 +60,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
> 
>  None of the other arch "measure" policy rules are conditional.  Should the
> new
> "measure" rule be limited?

This condition aims to avoid measuring the policy loaded even if a signed policy
is required. In that case, it would not be possible to directly write the policy
in the securityfs file.

> >  	"measure func=MODULE_CHECK",
> >  	NULL
> > diff --git a/security/integrity/ima/ima_fs.c
> > b/security/integrity/ima/ima_fs.c
> > index 012a58959ff0..75cb308cf01f 100644
> > --- a/security/integrity/ima/ima_fs.c
> > +++ b/security/integrity/ima/ima_fs.c
> > @@ -476,6 +476,7 @@ static int ima_release_policy(struct inode *inode,
> > struct file *file)
> >  	}
> >  
> >  	ima_update_policy();
> > +	ima_measure_loaded_policy();
> >  #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 bf2d7ba4c14a..e0b4dae922b6 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"
> > @@ -2022,7 +2023,6 @@ const char *const func_tokens[] = {
> >  	__ima_hooks(__ima_hook_stringify)
> >  };
> >  
> > -#ifdef	CONFIG_IMA_READ_POLICY
> 
> Removing the ifdef, here, does not affect viewing the IMA measurement lists,
> but
> allows copying and measuring the policy rules.  Please include a comment in
> the
> patch description.

Sure, will add in the next version.

> >  enum {
> >  	mask_exec = 0, mask_write, mask_read, mask_append
> >  };
> > @@ -2324,7 +2324,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)
> >  /*
> > @@ -2381,3 +2380,55 @@ bool ima_appraise_signature(enum kernel_read_file_id
> > id)
> >  	return found;
> >  }
> >  #endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
> > +
> 
> Please add kernel-doc here, something like:
> 
> /**
>  * 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.
>  */

Sure, thank you. If it is ok for you I can directly add what you suggested.

> > +void ima_measure_loaded_policy(void)
> > +{
> > +	const char *event_name = "ima_policy_loaded";
> > +	const char *op = "measure_loaded_ima_policy";
> > +	const char *audit_cause = "ENOMEM";
> > +	struct ima_rule_entry *rule_entry;
> > +	struct list_head *ima_rules_tmp;
> > +	struct seq_file file;
> > +	int result = -ENOMEM;
> > +	size_t file_len;
> > +	char rule[255];
> 
> The 255-byte buffer may be insufficient for custom policy rules that include
> additional fields such as LSM labels and other file metadata, unlike the
> simpler
> built-in and architecture-specific rules. Please increase the buffer size to
> accommodate the worst-case serialized rule length.

Yes, I wrongly took as reference the arch policy rules case. I don't know if the
worst-case can be precisely estimated. I could increase the buffer size and
check in any case if seq_has_overflowed(). Could it be an idea?

> > +
> > +	/* calculate IMA policy rules memory size */
> > +	file.buf = rule;
> > +	file.read_pos = 0;
> > +	file.size = 255;
> > +	file.count = 0;
> > +
> 
> Please add "lockdep_assert_held(&ima_write_mutex);"  here.

Yes, and this would actually fail because I'm not acquiring ima_write_mutex in
ima_release_policy().

> > +	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);
> > +		file_len += file.count;
> > +		file.count = 0;
> > +	}
> 
> Variables defined on the stack need to be initialized before being used.
> Please
> iniitalize file_len to zero.

Sure, will fix that.

Thank you,

Enrico

> > +	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, audit_cause, 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);
> > +}
> 
> Thanks,
> 
> Mimi

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

[Mimi Zohar]

On Thu, 2026-06-11 at 12:51 +0000, Enrico  Bravi wrote:
> 
> > > diff --git a/security/integrity/ima/ima_efi.c
> > > b/security/integrity/ima/ima_efi.c
> > > index 138029bfcce1..8e9f85ec9a86 100644
> > > --- a/security/integrity/ima/ima_efi.c
> > > +++ b/security/integrity/ima/ima_efi.c
> > > @@ -60,6 +60,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
> > 
> >  None of the other arch "measure" policy rules are conditional.  Should the
> > new
> > "measure" rule be limited?
> 
> This condition aims to avoid measuring the policy loaded even if a signed policy
> is required. In that case, it would not be possible to directly write the policy
> in the securityfs file.

Good point. Since it is different than the other rules, could you add a comment
here or in the patch description.

> 
> > >  	"measure func=MODULE_CHECK",
> > >  	NULL
> > > diff --git a/security/integrity/ima/ima_fs.c
> > > b/security/integrity/ima/ima_fs.c
> > > index 012a58959ff0..75cb308cf01f 100644
> > > --- a/security/integrity/ima/ima_fs.c
> > > +++ b/security/integrity/ima/ima_fs.c
> > > 
> > > @@ -2381,3 +2380,55 @@ bool ima_appraise_signature(enum kernel_read_file_id
> > > id)
> > >  	return found;
> > >  }
> > >  #endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
> > > +
> > 
> > Please add kernel-doc here, something like:
> > 
> > /**
> >  * 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.
> >  */
> 
> Sure, thank you. If it is ok for you I can directly add what you suggested.

This was suggested by Claude, so it should be acceptable to use.

> 
> > > +void ima_measure_loaded_policy(void)
> > > +{
> > > +	const char *event_name = "ima_policy_loaded";
> > > +	const char *op = "measure_loaded_ima_policy";
> > > +	const char *audit_cause = "ENOMEM";
> > > +	struct ima_rule_entry *rule_entry;
> > > +	struct list_head *ima_rules_tmp;
> > > +	struct seq_file file;
> > > +	int result = -ENOMEM;
> > > +	size_t file_len;
> > > +	char rule[255];
> > 
> > The 255-byte buffer may be insufficient for custom policy rules that include
> > additional fields such as LSM labels and other file metadata, unlike the
> > simpler
> > built-in and architecture-specific rules. Please increase the buffer size to
> > accommodate the worst-case serialized rule length.
> 
> Yes, I wrongly took as reference the arch policy rules case. I don't know if the
> worst-case can be precisely estimated. I could increase the buffer size and
> check in any case if seq_has_overflowed(). Could it be an idea?

Sounds good.

> 
> > > +
> > > +	/* calculate IMA policy rules memory size */
> > > +	file.buf = rule;
> > > +	file.read_pos = 0;
> > > +	file.size = 255;
> > > +	file.count = 0;
> > > +
> > 
> > Please add "lockdep_assert_held(&ima_write_mutex);"  here.
> 
> Yes, and this would actually fail because I'm not acquiring ima_write_mutex in
> ima_release_policy().

Thanks.

Mimi
> > 

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

[Enrico Bravi]

Hi Mimi,

On Thu, 2026-06-11 at 10:30 -0400, Mimi Zohar wrote:
> On Thu, 2026-06-11 at 12:51 +0000, Enrico  Bravi wrote:
> > 
> > > > diff --git a/security/integrity/ima/ima_efi.c
> > > > b/security/integrity/ima/ima_efi.c
> > > > index 138029bfcce1..8e9f85ec9a86 100644
> > > > --- a/security/integrity/ima/ima_efi.c
> > > > +++ b/security/integrity/ima/ima_efi.c
> > > > @@ -60,6 +60,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
> > > 
> > >  None of the other arch "measure" policy rules are conditional.  Should
> > > the
> > > new
> > > "measure" rule be limited?
> > 
> > This condition aims to avoid measuring the policy loaded even if a signed
> > policy
> > is required. In that case, it would not be possible to directly write the
> > policy
> > in the securityfs file.
> 
> Good point. Since it is different than the other rules, could you add a
> comment
> here or in the patch description.

Sure, will do.

> > 
> > > >  	"measure func=MODULE_CHECK",
> > > >  	NULL
> > > > diff --git a/security/integrity/ima/ima_fs.c
> > > > b/security/integrity/ima/ima_fs.c
> > > > index 012a58959ff0..75cb308cf01f 100644
> > > > --- a/security/integrity/ima/ima_fs.c
> > > > +++ b/security/integrity/ima/ima_fs.c
> > > > 
> > > > @@ -2381,3 +2380,55 @@ bool ima_appraise_signature(enum
> > > > kernel_read_file_id
> > > > id)
> > > >  	return found;
> > > >  }
> > > >  #endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
> > > > +
> > > 
> > > Please add kernel-doc here, something like:
> > > 
> > > /**
> > >  * 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.
> > >  */
> > 
> > Sure, thank you. If it is ok for you I can directly add what you suggested.
> 
> This was suggested by Claude, so it should be acceptable to use.
> 
> > 
> > > > +void ima_measure_loaded_policy(void)
> > > > +{
> > > > +	const char *event_name = "ima_policy_loaded";
> > > > +	const char *op = "measure_loaded_ima_policy";
> > > > +	const char *audit_cause = "ENOMEM";
> > > > +	struct ima_rule_entry *rule_entry;
> > > > +	struct list_head *ima_rules_tmp;
> > > > +	struct seq_file file;
> > > > +	int result = -ENOMEM;
> > > > +	size_t file_len;
> > > > +	char rule[255];
> > > 
> > > The 255-byte buffer may be insufficient for custom policy rules that
> > > include
> > > additional fields such as LSM labels and other file metadata, unlike the
> > > simpler
> > > built-in and architecture-specific rules. Please increase the buffer size
> > > to
> > > accommodate the worst-case serialized rule length.
> > 
> > Yes, I wrongly took as reference the arch policy rules case. I don't know if
> > the
> > worst-case can be precisely estimated. I could increase the buffer size and
> > check in any case if seq_has_overflowed(). Could it be an idea?
> 
> Sounds good.

Perfect, thank you.

Enrico

> > 
> > > > +
> > > > +	/* calculate IMA policy rules memory size */
> > > > +	file.buf = rule;
> > > > +	file.read_pos = 0;
> > > > +	file.size = 255;
> > > > +	file.count = 0;
> > > > +
> > > 
> > > Please add "lockdep_assert_held(&ima_write_mutex);"  here.
> > 
> > Yes, and this would actually fail because I'm not acquiring ima_write_mutex
> > in
> > ima_release_policy().
> 
> Thanks.
> 
> Mimi

[RFC][PATCH v3 2/2] ima: measure buffer sent to securityfs policy file

[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

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.

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 a223d3f30d88..320c80a1a847 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -426,6 +426,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 75cb308cf01f..601718e02429 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -362,6 +362,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 1d6229b156fb..174110da0030 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -1204,6 +1204,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 -ENOPARAM;
+
+	return process_buffer_measurement(&nop_mnt_idmap, NULL, buf, buf_len,
+					 "ima_write_policy_buf", 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 e0b4dae922b6..d44af7422c04 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

Re: [RFC][PATCH v3 2/2] ima: measure buffer sent to securityfs policy file

[Mimi Zohar]

On Tue, 2026-05-26 at 15:51 +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:
> 
> 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.

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

Enrco, Roberto, a reason for measuring invalid or malformed IMA policy rules
needs to be provided here.

In addition to the "ima_policy" critical data, why is this mechanism needed?


> The
> measurement template is forced to ima-buf.

Please include directions for verifying the measurement record here in the patch
description.

> 
> 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 a223d3f30d88..320c80a1a847 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -426,6 +426,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 75cb308cf01f..601718e02429 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -362,6 +362,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 1d6229b156fb..174110da0030 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -1204,6 +1204,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 -ENOPARAM;

Please return -EINVAL.


> +
> +	return process_buffer_measurement(&nop_mnt_idmap, NULL, buf, buf_len,
> +					 "ima_write_policy_buf", POLICY_CHECK,
> +					 0, NULL, false, NULL, 0);

Parallel to "ima_policy_loaded" consider naming the record as
"ima_policy_written".

The indentation is off by a character.

> +}
> +
>  #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
>  
>  /**

[...]

thanks,

Mimi

Re: [RFC][PATCH v3 2/2] ima: measure buffer sent to securityfs policy file

[Roberto Sassu]

On Wed, 2026-06-10 at 14:59 -0400, Mimi Zohar wrote:
> On Tue, 2026-05-26 at 15:51 +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:
> > 
> > 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.
> 
> > 
> > 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).
> 
> Enrco, Roberto, a reason for measuring invalid or malformed IMA policy rules
> needs to be provided here.

One reason would be to be able to detect attempts to corrupt IMA by
loading malformed data, since the measurement is performed before the
policy is parsed.

> In addition to the "ima_policy" critical data, why is this mechanism needed?

POLICY_CHECK already measures partial policy load by file. This patch
would just complete the POLICY_CHECK hook by measuring policy load
by buffer.

I believe they can both cohexist, both would allow to know which policy
is loaded at any time. With the critical data, the existing policy is
included in every entry, but it is easier to derive the current policy
loaded. It would be up to the remote attestation solution to determine
which solution is more suitable.

Roberto

> > The
> > measurement template is forced to ima-buf.
> 
> Please include directions for verifying the measurement record here in the patch
> description.
> 
> > 
> > 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 a223d3f30d88..320c80a1a847 100644
> > --- a/security/integrity/ima/ima.h
> > +++ b/security/integrity/ima/ima.h
> > @@ -426,6 +426,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 75cb308cf01f..601718e02429 100644
> > --- a/security/integrity/ima/ima_fs.c
> > +++ b/security/integrity/ima/ima_fs.c
> > @@ -362,6 +362,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 1d6229b156fb..174110da0030 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -1204,6 +1204,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 -ENOPARAM;
> 
> Please return -EINVAL.
> 
> 
> > +
> > +	return process_buffer_measurement(&nop_mnt_idmap, NULL, buf, buf_len,
> > +					 "ima_write_policy_buf", POLICY_CHECK,
> > +					 0, NULL, false, NULL, 0);
> 
> Parallel to "ima_policy_loaded" consider naming the record as
> "ima_policy_written".
> 
> The indentation is off by a character.
> 
> > +}
> > +
> >  #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
> >  
> >  /**
> 
> [...]
> 
> thanks,
> 
> Mimi
>