Re: [Patch V1] ima: avoid duplicate policy rules insertions

[Roberto Sassu <roberto.sassu@huaweicloud.com>]

On Thu, 2025-11-06 at 18:14 +0000, Tahera Fahimi wrote:
> Prevent redundant IMA policy rules by checking for duplicates before insertion. This ensures that
> rules are not re-added when userspace is restarted (using systemd-soft-reboot) without a full system
> reboot. ima_rule_exists() detects duplicates in both temporary and active rule lists.

+ Lennart

Hi Tahera

thanks for the patch!


Wouldn't be better to enhance systemd-soft-reboot to not send the same
IMA policy again?

Thanks

Roberto

> Signed-off-by: Tahera Fahimi <taherafahimi@linux.microsoft.com>
> ---
>  security/integrity/ima/ima_policy.c | 157 +++++++++++++++++++++++++++-
>  1 file changed, 156 insertions(+), 1 deletion(-)
> 
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 164d62832f8ec..3dd902101dbda 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -1953,6 +1953,153 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  	return result;
>  }
>  
> +static bool template_has_field(const char *field_id, const struct ima_template_desc *template2)
> +{
> +	int j;
> +
> +	for (int j = 0; j < template2->num_fields; j++)
> +		if (strcmp(field_id, template2->fields[j]->field_id) == 0)
> +			return true;
> +
> +	return false;
> +}
> +
> +static bool keyring_has_item(const char *item, const struct ima_rule_opt_list *keyrings)
> +{
> +	int j;
> +
> +	for (j = 0; j < keyrings->count; j++) {
> +		if (strcmp(item, keyrings->items[j]) == 0)
> +			return true;
> +	}
> +	return false;
> +}
> +
> +static bool labels_has_item(const char *item, const struct ima_rule_opt_list *labels)
> +{
> +	int j;
> +
> +	for (j = 0; j < labels->count; j++) {
> +		if (strcmp(item, labels->items[j]) == 0)
> +			return true;
> +	}
> +	return false;
> +}
> +
> +static bool ima_rules_equal(const struct ima_rule_entry *rule1, const struct ima_rule_entry *rule2)
> +{
> +	int i;
> +
> +	if (rule1->flags != rule2->flags)
> +		return false;
> +
> +	if (rule1->action != rule2->action)
> +		return false;
> +
> +	if (((rule1->flags & IMA_FUNC) && rule1->func != rule2->func) ||
> +	    ((rule1->flags & (IMA_MASK | IMA_INMASK)) && rule1->mask != rule2->mask) ||
> +	    ((rule1->flags & IMA_FSMAGIC) && rule1->fsmagic != rule2->fsmagic) ||
> +	    ((rule1->flags & IMA_FSUUID) && !uuid_equal(&rule1->fsuuid, &rule2->fsuuid)) ||
> +	    ((rule1->flags & IMA_UID) && !uid_eq(rule1->uid, rule2->uid)) ||
> +	    ((rule1->flags & IMA_GID) && !gid_eq(rule1->gid, rule2->gid)) ||
> +	    ((rule1->flags & IMA_FOWNER) && !uid_eq(rule1->fowner, rule2->fowner)) ||
> +	    ((rule1->flags & IMA_FGROUP) && !gid_eq(rule1->fgroup, rule2->fgroup)) ||
> +	    ((rule1->flags & IMA_FSNAME) && (strcmp(rule1->fsname, rule2->fsname) != 0)) ||
> +	    ((rule1->flags & IMA_PCR) && rule1->pcr != rule2->pcr) ||
> +	    ((rule1->flags & IMA_VALIDATE_ALGOS) &&
> +	      rule1->allowed_algos != rule2->allowed_algos) ||
> +	    ((rule1->flags & IMA_EUID) && !uid_eq(rule1->uid, rule2->uid)) ||
> +	    ((rule1->flags & IMA_EGID) && !gid_eq(rule1->gid, rule2->gid)))
> +		return false;
> +
> +	if (!rule1->template && !rule2->template) {
> +		;
> +	} else if (!rule1->template || !rule2->template) {
> +		return false;
> +	} else if (rule1->template->num_fields != rule2->template->num_fields) {
> +		return false;
> +	} else if (rule1->template->num_fields != 0) {
> +		for (i = 0; i < rule1->template->num_fields; i++) {
> +			if (!template_has_field(rule1->template->fields[i]->field_id,
> +						rule2->template))
> +				return false;
> +		}
> +	}
> +
> +	if (rule1->flags & IMA_KEYRINGS) {
> +		if (!rule1->keyrings && !rule2->keyrings) {
> +			;
> +		} else if (!rule1->keyrings || !rule2->keyrings) {
> +			return false;
> +		} else if (rule1->keyrings->count != rule2->keyrings->count) {
> +			return false;
> +		} else if (rule1->keyrings->count != 0) {
> +			for (i = 0; i < rule1->keyrings->count; i++) {
> +				if (!keyring_has_item(rule1->keyrings->items[i], rule2->keyrings))
> +					return false;
> +			}
> +		}
> +	}
> +
> +	if (rule1->flags & IMA_LABEL) {
> +		if (!rule1->label && !rule2->label) {
> +			;
> +		} else if (!rule1->label || !rule2->label) {
> +			return false;
> +		} else if (rule1->label->count != rule2->label->count) {
> +			return false;
> +		} else if (rule1->label->count != 0) {
> +			for (i = 0; i < rule1->label->count; i++) {
> +				if (!labels_has_item(rule1->label->items[i], rule2->label))
> +					return false;
> +			}
> +		}
> +	}
> +
> +	for (i = 0; i < MAX_LSM_RULES; i++) {
> +		if (!rule1->lsm[i].rule && !rule2->lsm[i].rule)
> +			continue;
> +
> +		if (!rule1->lsm[i].rule || !rule2->lsm[i].rule)
> +			return false;
> +
> +		if (strcmp(rule1->lsm[i].args_p, rule2->lsm[i].args_p) != 0)
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
> +/**
> + * ima_rule_exists - check if a rule already exists in the policy
> + *
> + * Checking both the active policy and the temporary rules list.
> + */
> +static bool ima_rule_exists(struct ima_rule_entry *new_rule)
> +{
> +	struct ima_rule_entry *entry;
> +	struct list_head *ima_rules_tmp;
> +
> +	if (!list_empty(&ima_temp_rules)) {
> +		list_for_each_entry(entry, &ima_temp_rules, list) {
> +			if (ima_rules_equal(entry, new_rule))
> +				return true;
> +		}
> +	}
> +
> +	rcu_read_lock();
> +	ima_rules_tmp = rcu_dereference(ima_rules);
> +	list_for_each_entry_rcu(entry, ima_rules_tmp, list) {
> +		if (ima_rules_equal(entry, new_rule)) {
> +			rcu_read_unlock();
> +			return true;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return false;
> +}
> +
>  /**
>   * ima_parse_add_rule - add a rule to ima_policy_rules
>   * @rule: ima measurement policy rule
> @@ -1993,7 +2140,15 @@ ssize_t ima_parse_add_rule(char *rule)
>  		return result;
>  	}
>  
> -	list_add_tail(&entry->list, &ima_temp_rules);
> +	if (!ima_rule_exists(entry)) {
> +		list_add_tail(&entry->list, &ima_temp_rules);
> +	} else {
> +		result = -EEXIST;
> +		ima_free_rule(entry);
> +		integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
> +				    NULL, op, "duplicate-policy", result,
> +				    audit_info);
> +	}
>  
>  	return len;
>  }