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 <[email protected]>
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
