On 2022/9/20 5:35, Mimi Zohar wrote:
Hi Scott,
@@ -612,6 +614,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
else
return false;
}
+
+retry:
switch (i) {
case LSM_OBJ_USER:
case LSM_OBJ_ROLE:
@@ -631,10 +635,28 @@ static bool ima_match_rules(struct ima_rule_entry *rule,
default:
break;
}
- if (!rc)
- return false;
+
+ if (rc == -ESTALE) {
+ rule = ima_lsm_copy_rule(rule);
Re-using rule here
I'll use another variable here.
+ if (rule) {
and here doesn't look right.
What seems to be wrong here? ima_lsm_copy_rule() returns a shallow copy
of the rule, and NULL if the copy fails. Only if the returned rule is
not NULL should we proceed with the retry. I used rule_reinitialized to
memorize whether the current rule is copied so that we should free it
later on.
+ rule_reinitialized = true;
+ goto retry;
+ }
+ }
+ if (!rc) {
+ result = false;
+ goto out;
+ }
}
- return true;
+ result = true;
+
+out:
+ if (rule_reinitialized) {
+ for (i = 0; i < MAX_LSM_RULES; i++)
+ ima_filter_rule_free(rule->lsm[i].rule);
+ kfree(rule);
+ }
Shouldn't freeing the memory be immediately after the retry?
Otherwise, only the last instance of processing -ESTALE would be freed.
ima_lsm_copy_rule() would update every member of rule->lsm, and the
retry is within the for loop on members of rule->lsm. We'd better keep
the copied rule till the loop ends. To avoid race condition if the LSM
rule has been updated again during the loop, I can add a guard here.
+ return result;
}
/*
--
Best
GUO Zihua
