[PATCH] ima: prevent concurrent list operations in ima_lsm_update_rules

[PATCH] ima: prevent concurrent list operations in ima_lsm_update_rules

[Zhao Yipeng]

The current implementation of IMA policy list replacement via
list_replace_rcu may trigger general protection faults under concurrent
load policy operations. This occurs when a process replaces a node in
ima_policy_rules list and sets old->prev = LIST_POISON2, while another
parallel process still holds references to the old node. Subsequent list
operations on the poisoned pointer result in kernel panic due to invalid
memory access.

To resolve this, introduce a mutex lock (ima_rules_mutex) in
ima_lsm_update_rules() to protect. ima_update_policy() also use the
ima_policy_rules. Introduce a mutex lock in it.

Fixes: b16942455193 ("ima: use the lsm policy update notifier")
Signed-off-by: Zhao Yipeng <zhaoyipeng5@huawei.com>
---
 security/integrity/ima/ima_policy.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 128fab897930..d27e615e97d5 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -471,6 +471,8 @@ static bool ima_rule_contains_lsm_cond(struct ima_rule_entry *entry)
 	return false;
 }
 
+static DEFINE_MUTEX(ima_rules_mutex);
+
 /*
  * The LSM policy can be reloaded, leaving the IMA LSM based rules referring
  * to the old, stale LSM policy.  Update the IMA LSM based rules to reflect
@@ -481,16 +483,19 @@ static void ima_lsm_update_rules(void)
 	struct ima_rule_entry *entry, *e;
 	int result;
 
+	mutex_lock(&ima_rules_mutex);
 	list_for_each_entry_safe(entry, e, &ima_policy_rules, list) {
 		if (!ima_rule_contains_lsm_cond(entry))
 			continue;
 
 		result = ima_lsm_update_rule(entry);
 		if (result) {
+			mutex_unlock(&ima_rules_mutex);
 			pr_err("lsm rule update error %d\n", result);
 			return;
 		}
 	}
+	mutex_unlock(&ima_rules_mutex);
 }
 
 int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
@@ -1038,9 +1043,12 @@ int ima_check_policy(void)
  */
 void ima_update_policy(void)
 {
-	struct list_head *policy = &ima_policy_rules;
+	struct list_head *policy;
 
+	mutex_lock(&ima_rules_mutex);
+	policy = &ima_policy_rules;
 	list_splice_tail_init_rcu(&ima_temp_rules, policy, synchronize_rcu);
+	mutex_unlock(&ima_rules_mutex);
 
 	if (ima_rules != (struct list_head __rcu *)policy) {
 		ima_policy_flag = 0;
-- 
2.34.1

Re: [PATCH] ima: prevent concurrent list operations in ima_lsm_update_rules

[Mimi Zohar]

On Tue, 2025-05-27 at 20:51 +0800, Zhao Yipeng wrote:
> The current implementation of IMA policy list replacement via
> list_replace_rcu may trigger general protection faults under concurrent
> load policy operations. This occurs when a process replaces a node in
> ima_policy_rules list and sets old->prev = LIST_POISON2, while another
> parallel process still holds references to the old node. Subsequent list
> operations on the poisoned pointer result in kernel panic due to invalid
> memory access.
> 
> To resolve this, introduce a mutex lock (ima_rules_mutex) in
> ima_lsm_update_rules() to protect. ima_update_policy() also use the
> ima_policy_rules. Introduce a mutex lock in it.

A new IMA policy may replace the existing builtin policy rules with a custom
policy. In all other cases, the IMA policy rules may only be extended.  Writing
or extending the IMA policy rules requires taking the ima_write_mutex.

There's no need for a new mutex.

Mimi

> 
> Fixes: b16942455193 ("ima: use the lsm policy update notifier")
> Signed-off-by: Zhao Yipeng <zhaoyipeng5@huawei.com>
> ---
>  security/integrity/ima/ima_policy.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/security/integrity/ima/ima_policy.c
> b/security/integrity/ima/ima_policy.c
> index 128fab897930..d27e615e97d5 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -471,6 +471,8 @@ static bool ima_rule_contains_lsm_cond(struct
> ima_rule_entry *entry)
>  	return false;
>  }
>  
> +static DEFINE_MUTEX(ima_rules_mutex);
> +
>  /*
>   * The LSM policy can be reloaded, leaving the IMA LSM based rules referring
>   * to the old, stale LSM policy.  Update the IMA LSM based rules to reflect
> @@ -481,16 +483,19 @@ static void ima_lsm_update_rules(void)
>  	struct ima_rule_entry *entry, *e;
>  	int result;
>  
> +	mutex_lock(&ima_rules_mutex);
>  	list_for_each_entry_safe(entry, e, &ima_policy_rules, list) {
>  		if (!ima_rule_contains_lsm_cond(entry))
>  			continue;
>  
>  		result = ima_lsm_update_rule(entry);
>  		if (result) {
> +			mutex_unlock(&ima_rules_mutex);
>  			pr_err("lsm rule update error %d\n", result);
>  			return;
>  		}
>  	}
> +	mutex_unlock(&ima_rules_mutex);
>  }
>  
>  int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
> @@ -1038,9 +1043,12 @@ int ima_check_policy(void)
>   */
>  void ima_update_policy(void)
>  {
> -	struct list_head *policy = &ima_policy_rules;
> +	struct list_head *policy;
>  
> +	mutex_lock(&ima_rules_mutex);
> +	policy = &ima_policy_rules;
>  	list_splice_tail_init_rcu(&ima_temp_rules, policy, synchronize_rcu);
> +	mutex_unlock(&ima_rules_mutex);
>  
>  	if (ima_rules != (struct list_head __rcu *)policy) {
>  		ima_policy_flag = 0;