diff mbox series

[4.19,v3,6/6] ima: Handle -ESTALE returned by ima_filter_rule_match()

Message ID 20230228080630.52370-7-guozihua@huawei.com (mailing list archive)
State Not Applicable
Headers show
Series Backport handling -ESTALE policy update failure to 4.19 | expand

Commit Message

Guozihua (Scott) Feb. 28, 2023, 8:06 a.m. UTC
[ Upstream commit c7423dbdbc9ecef7fff5239d144cad4b9887f4de ]

IMA relies on the blocking LSM policy notifier callback to update the
LSM based IMA policy rules.

When SELinux update its policies, IMA would be notified and starts
updating all its lsm rules one-by-one. During this time, -ESTALE would
be returned by ima_filter_rule_match() if it is called with a LSM rule
that has not yet been updated. In ima_match_rules(), -ESTALE is not
handled, and the LSM rule is considered a match, causing extra files
to be measured by IMA.

Fix it by re-initializing a temporary rule if -ESTALE is returned by
ima_filter_rule_match(). The origin rule in the rule list would be
updated by the LSM policy notifier callback.

Fixes: b16942455193 ("ima: use the lsm policy update notifier")
Signed-off-by: GUO Zihua <guozihua@huawei.com>
Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: GUO Zihua <guozihua@huawei.com>
---
 security/integrity/ima/ima_policy.c | 40 ++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 9 deletions(-)

Comments

Guozihua (Scott) April 17, 2023, 11:39 a.m. UTC | #1
On 2023/2/28 16:06, GUO Zihua wrote:
> [ Upstream commit c7423dbdbc9ecef7fff5239d144cad4b9887f4de ]
> 
> IMA relies on the blocking LSM policy notifier callback to update the
> LSM based IMA policy rules.
> 
> When SELinux update its policies, IMA would be notified and starts
> updating all its lsm rules one-by-one. During this time, -ESTALE would
> be returned by ima_filter_rule_match() if it is called with a LSM rule
> that has not yet been updated. In ima_match_rules(), -ESTALE is not
> handled, and the LSM rule is considered a match, causing extra files
> to be measured by IMA.
> 
> Fix it by re-initializing a temporary rule if -ESTALE is returned by
> ima_filter_rule_match(). The origin rule in the rule list would be
> updated by the LSM policy notifier callback.
> 
> Fixes: b16942455193 ("ima: use the lsm policy update notifier")
> Signed-off-by: GUO Zihua <guozihua@huawei.com>
> Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: GUO Zihua <guozihua@huawei.com>
> ---
>  security/integrity/ima/ima_policy.c | 40 ++++++++++++++++++++++-------
>  1 file changed, 31 insertions(+), 9 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 5256ff008f11..e9e15e622cf2 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -374,6 +374,9 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>  			    enum ima_hooks func, int mask)
>  {
>  	int i;
> +	bool result = false;
> +	struct ima_rule_entry *lsm_rule = rule;
> +	bool rule_reinitialized = false;
>  
>  	if ((rule->flags & IMA_FUNC) &&
>  	    (rule->func != func && func != POST_SETATTR))
> @@ -412,38 +415,57 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>  		int rc = 0;
>  		u32 osid;
>  
> -		if (!rule->lsm[i].rule) {
> -			if (!rule->lsm[i].args_p)
> +		if (!lsm_rule->lsm[i].rule) {
> +			if (!lsm_rule->lsm[i].args_p)
>  				continue;
>  			else
>  				return false;
>  		}
> +
> +retry:
>  		switch (i) {
>  		case LSM_OBJ_USER:
>  		case LSM_OBJ_ROLE:
>  		case LSM_OBJ_TYPE:
>  			security_inode_getsecid(inode, &osid);
>  			rc = security_filter_rule_match(osid,
> -							rule->lsm[i].type,
> +							lsm_rule->lsm[i].type,
>  							Audit_equal,
> -							rule->lsm[i].rule,
> +							lsm_rule->lsm[i].rule,
>  							NULL);
>  			break;
>  		case LSM_SUBJ_USER:
>  		case LSM_SUBJ_ROLE:
>  		case LSM_SUBJ_TYPE:
>  			rc = security_filter_rule_match(secid,
> -							rule->lsm[i].type,
> +							lsm_rule->lsm[i].type,
>  							Audit_equal,
> -							rule->lsm[i].rule,
> +							lsm_rule->lsm[i].rule,
>  							NULL);
>  		default:
>  			break;
>  		}
> -		if (!rc)
> -			return false;
> +
> +		if (rc == -ESTALE && !rule_reinitialized) {
> +			lsm_rule = ima_lsm_copy_rule(rule);
> +			if (lsm_rule) {
> +				rule_reinitialized = true;
> +				goto retry;
> +			}
> +		}
> +		if (!rc) {
> +			result = false;
> +			goto out;
> +		}
> +	}
> +	result = true;
> +
> +out:
> +	if (rule_reinitialized) {
> +		ima_lsm_free_rule(lsm_rule);
> +		kfree(lsm_rule);
>  	}
> -	return true;
> +	return result;
>  }
>  
>  /*

Ping?
diff mbox series

Patch

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 5256ff008f11..e9e15e622cf2 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -374,6 +374,9 @@  static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
 			    enum ima_hooks func, int mask)
 {
 	int i;
+	bool result = false;
+	struct ima_rule_entry *lsm_rule = rule;
+	bool rule_reinitialized = false;
 
 	if ((rule->flags & IMA_FUNC) &&
 	    (rule->func != func && func != POST_SETATTR))
@@ -412,38 +415,57 @@  static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
 		int rc = 0;
 		u32 osid;
 
-		if (!rule->lsm[i].rule) {
-			if (!rule->lsm[i].args_p)
+		if (!lsm_rule->lsm[i].rule) {
+			if (!lsm_rule->lsm[i].args_p)
 				continue;
 			else
 				return false;
 		}
+
+retry:
 		switch (i) {
 		case LSM_OBJ_USER:
 		case LSM_OBJ_ROLE:
 		case LSM_OBJ_TYPE:
 			security_inode_getsecid(inode, &osid);
 			rc = security_filter_rule_match(osid,
-							rule->lsm[i].type,
+							lsm_rule->lsm[i].type,
 							Audit_equal,
-							rule->lsm[i].rule,
+							lsm_rule->lsm[i].rule,
 							NULL);
 			break;
 		case LSM_SUBJ_USER:
 		case LSM_SUBJ_ROLE:
 		case LSM_SUBJ_TYPE:
 			rc = security_filter_rule_match(secid,
-							rule->lsm[i].type,
+							lsm_rule->lsm[i].type,
 							Audit_equal,
-							rule->lsm[i].rule,
+							lsm_rule->lsm[i].rule,
 							NULL);
 		default:
 			break;
 		}
-		if (!rc)
-			return false;
+
+		if (rc == -ESTALE && !rule_reinitialized) {
+			lsm_rule = ima_lsm_copy_rule(rule);
+			if (lsm_rule) {
+				rule_reinitialized = true;
+				goto retry;
+			}
+		}
+		if (!rc) {
+			result = false;
+			goto out;
+		}
+	}
+	result = true;
+
+out:
+	if (rule_reinitialized) {
+		ima_lsm_free_rule(lsm_rule);
+		kfree(lsm_rule);
 	}
-	return true;
+	return result;
 }
 
 /*